Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
> > So I'm unsure if it is a good idea to include those patches. > > They are harmless, but according to what Melchior has pointed out, some of > the code (what I added to the exceptions classes) is redundant (basically, > what is written there is auto-generated by the compiler unless it has > problems). For the other parts of the patch, it is still relevant. > I'm going to rework it later by eliminating the above redundancy when > I have time (probably this weekend). Actually, I've looked at them now myself, and I ask the patching powers that be to apply it, save for the redundant parts patching the files SimGear/source/simgear/structure/exception.cxx and SimGear/source/simgear/structure/exception.hxx for which Melchior has voiced rightful objection. The 2nd revision of the patch is just a cleanup of some exception throwing and catching, sometimes providing additional diagnostics (e.g., the mysterious "Failed to open file" is now augmented with the actual file location caught). Vassilii ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
2005/11/28, Vassilii Khachaturov <[EMAIL PROTECTED]>: > problems). For the other parts of the patch, it is still relevant. > I'm going to rework it later by eliminating the above redundancy when > I have time (probably this weekend). That I was afraid of. That the new patch is on the way. ;-) > > If yes, how to test if it works well? > > Depends on what kind of testing you want to do. Either look at the > exceptions thrown and try to induce each one of them, or probably > just do the regular flying and see if it is still OK. Flying is OK. But I'm not sure where I have to look for exceptions. Into terminal? I didn't see anything before, neither after the patch was applied. > Also look at the code and see if you find something obviously stupid that > I've > overlooked. Please don't rely on me. I'm totally beginner programer. > linux/Pentium IV 32 bit. So we have the same platform. So I didn't catch the point of > Since you are running on a different platform than I do > (I have the errno there), I would like to ask you to run the exception > checking snippet in this thread and report the results, while I'm doing > the final testing of the shortened patch. Regards Ladislav. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
> So I'm unsure if it is a good idea to include those patches. They are harmless, but according to what Melchior has pointed out, some of the code (what I added to the exceptions classes) is redundant (basically, what is written there is auto-generated by the compiler unless it has problems). For the other parts of the patch, it is still relevant. I'm going to rework it later by eliminating the above redundancy when I have time (probably this weekend). > If yes, how to test if it works well? Depends on what kind of testing you want to do. Either look at the exceptions thrown and try to induce each one of them, or probably just do the regular flying and see if it is still OK. Also look at the code and see if you find something obviously stupid that I've overlooked. > BTW. Vassilij, what's your platform? I overlooked it, maybe. (Mine is > Linux on AMD 32bit) linux/Pentium IV 32 bit. Vassilii ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
So I'm unsure if it is a good idea to include those patches. If yes, how to test if it works well? BTW. Vassilij, what's your platform? I overlooked it, maybe. (Mine is Linux on AMD 32bit) Regards Ladislav. 2005/11/26, Vassilii Khachaturov <[EMAIL PROTECTED]>: > > thanks for catching this one. I'll 1) go to sleep and hope that > the flu will go away enough to get thinking sanely again 2) will try > not to submit lengthy patches coded up when being sick before > reviewing them myself being sane 3) re-read the BS book ASAP in > case I forgot smth else out of C++ > > V. > > > ___ > Flightgear-devel mailing list > Flightgear-devel@flightgear.org > http://mail.flightgear.org/mailman/listinfo/flightgear-devel > 2f585eeea02e2c79d7b1d8c4963bae2d > ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
Hi Melchior, thanks for the help. > * Vassilii Khachaturov -- Saturday 26 November 2005 11:02: > > > But ... classes without copy/assignment operator aren't copied > > > byte-by-byte, but member-by-member[1]. > > > It's a pity I am at home & sick, and without the book. I don't know > > what is written in the section you refer to. > > There's written what I said: that classes without copy/assignment > operator aren't copied byte-by-byte, but member-by-member. I'm > not going to cite the whole book. You have to trust me. :-P sure... Since now I see the code behaves that way, too :) > > Therefore the only > > thing I can do is try to code it and see what's happening. I tried to > > amend the snippet with a 3rd case, which crashes on my machine, > > No, it doesn't "crash your machine". It calls std::unexpected() > because you throw an exception that you *explicitly* disallowed. > That's a feature! :-) yeah, right... and I never added a handler catching EE in the catch loop, so it aborts even w/o the exception specification. Fixing those shows (e.g. by adding EE's inheritance from exception (as you suggested), and catching an exception& instead of E&) that everything works, and that the E's copy ctor does auto-fire when the EE gets cloned via the default copy ctor semantics - it's fixed at http://www.tarunz.org/~vassilii/locally-created-exceptions.cpp > > What do you think? > > I think that I should debug fgfs/sg, not your code snippets, ...which of course have no relation to fgfs/sg debugging :-))) > [snip] thanks for catching this one. I'll 1) go to sleep and hope that the flu will go away enough to get thinking sanely again 2) will try not to submit lengthy patches coded up when being sick before reviewing them myself being sane 3) re-read the BS book ASAP in case I forgot smth else out of C++ V. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
* Vassilii Khachaturov -- Saturday 26 November 2005 11:02:
> > But ... classes without copy/assignment operator aren't copied
> > byte-by-byte, but member-by-member[1].
> It's a pity I am at home & sick, and without the book. I don't know
> what is written in the section you refer to.
There's written what I said: that classes without copy/assignment
operator aren't copied byte-by-byte, but member-by-member. I'm
not going to cite the whole book. You have to trust me. :-P
> Therefore the only
> thing I can do is try to code it and see what's happening. I tried to
> amend the snippet with a 3rd case, which crashes on my machine,
No, it doesn't "crash your machine". It calls std::unexpected()
because you throw an exception that you *explicitly* disallowed.
That's a feature! :-)
> What do you think?
I think that I should debug fgfs/sg, not your code snippets,
but anyway:
> void foo(int i) throw(exception&)
Here you say that only class "exception" and its derivatives are
allowed.
> struct EE {
> E e;
> EE(const char* s) : e(s) { cout << "EE::EE(" << s << ")" << endl;}
> virtual ~EE() throw() { cout << "EE::~EE(" << e._msg << ")" << endl;}
> };
But you don't make EE a derivative of "exception":
struct EE : public exception { ...
m.
PS: FlightGear/SimGear doesn't use "Exception Specifications", so the
bug in your test program can't really happen.
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
> But ... classes without copy/assignment operator aren't copied
> byte-by-byte, but member-by-member[1]. So, for string members the
> string copy constructor is used. Again, the code looks right to me
> as it is.
>
> m.
>
>
> [0] Bjarne Stroustrup, "The C++ Programming Language", 2nd edition,
> p. 582, "r.12.8 Copying Class Objects"
>
It's a pity I am at home & sick, and without the book. I don't know
what is written in the section you refer to.
Therefore the only
thing I can do is try to code it and see what's happening. I tried to
amend the snippet with a 3rd case, which crashes on my machine, and,
AFAIU, precisely because the default member copy semantics is byte-by-byte
on my gcc (now it could well be that this is not the std, but I was pretty
happy about gcc compliance so far). What do you think?
BTW, maybe you meant to say that if I don't provide a copy ctor
in the derived class, then the parent's copy ctor is nevertheless
involved on the parent portion? I know *that*, but it doesn't cover
copying of the derived class' instance data.
#include
#include
#include
using namespace std;
struct E : exception {
E() : _msg("DEFAULT") { cout << "E::E()" << endl; }
E(const char* s) : _msg(s) {
cout << "E::E(const char*" << s << ")" << endl; }
E(const E& e) : _msg(e._msg + "(clone)") {
cout << "E::E(const E&" << e._msg << ")" << endl; }
E& operator=(const E& e) {
cout << "E::operator=(" << e._msg << ") assigned over " << _msg
<< endl;
_msg = e._msg + "(assigned)";
return *this;
}
virtual ~E() throw() { cout << "E::~E() " << _msg << endl; }
const char* what() const throw() { _msg.c_str(); }
string _msg;
};
struct EE {
E e;
EE(const char* s) : e(s) { cout << "EE::EE(" << s << ")" << endl;}
virtual ~EE() throw() { cout << "EE::~EE(" << e._msg << ")" << endl;}
};
void foo(int i) throw(exception&)
{
cout << "foo entering" << endl;
E unused("xUNUSED");
switch (i) {
case 0:
break;
case 1:
throw E("x1");
case 2: {
E weird("x2");
throw weird;
}
case 3: {
EE default_cloned("x3");
throw default_cloned; // CRASH as EE::e is never cloned
properly
}
}
cout << "foo exiting" << endl;
}
int main()
{
for (int i = 0; i < 4; i++) {
try {
foo(i);
}
catch (E& e) {
cout << "Caught " << e.what() << endl;
}
}
return 0;
}
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
* Vassilii Khachaturov -- Friday 25 November 2005 22:57: > The following are still in > > * the exception classes were lacking the copy ctors and assignment > > operators, but the default ones for them were unusable as the string > > instance members are not suitable for byte-by-byte copying! But ... classes without copy/assignment operator aren't copied byte-by-byte, but member-by-member[1]. So, for string members the string copy constructor is used. Again, the code looks right to me as it is. m. [0] Bjarne Stroustrup, "The C++ Programming Language", 2nd edition, p. 582, "r.12.8 Copying Class Objects" ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
I have re-worked the patch into a shorter one. > It has been split as follows: > > 1. > http://mail.flightgear.org/pipermail/flightgear-devel/2005-November/040968.html > SimGear-level changes Please see the attached simgear-except.diff for the new version of these, or http://www.tarunz.org/~vassilii/fg/simgear-except.diff > 2. > http://mail.flightgear.org/pipermail/flightgear-devel/2005-November/040969.html > FlightGear-level changes (except for the JSBSim code) See the attached flightgear-except.diff for the new version of these, or http://www.tarunz.org/~vassilii/fg/flightgear-except.diff > 3. > http://mail.flightgear.org/pipermail/flightgear-devel/2005-November/040970.html > JSBSim code changes (no bug fixed there, they're only pinpointed with > FIXME comments). I haven't included any JSBSim-related changes now at this stage. > What has been done in the patch: This item > * whenever an exception object was created on a stack and then thrown > thus causing the dtor for that object to fire!), it was replaced > with a STATIC exception object use in the same scope. I've > reviewed all the cases for the potential MT problems this might > create, and think that there's nothing dangerous, but I'd appreciate > your review of the code from this aspect. is N/A any longer, as per Melchior's quote of the BS C++ 2nd ed book The following things are still there > * in some cases more specific sg exception types were used in place > of the more generic one, e.g., sg_io_exception instead of sg_exception > when the context of the error was an IO error > * in some cases, the error message was made more specific except for this one > * in a couple of cases, I fetched the IO error string via strerror, > knowingly pulling in bogus data in case another thread does an IO call > at the same moment. These are marked with a FIXME. The following are still in > * the exception classes were lacking the copy ctors and assignment > operators, but the default ones for them were unusable as the string > instance members are not suitable for byte-by-byte copying! See the > PropsVisitor::setException for an example of a faulty use that > is no longer broken because of this change. > * minor style fix for exception rethrowing --- using throw; whenever > a re-throw is made; sometimes optimizing away the exception symbol name > in the catch handler at all > * more specific catch handlers added in some places -- e.g., > an sg_io_exception caught ahead of sg_exception > as is my request below > Please review, test, comment and apply! (don't forget to revert the older larger patch if you've already started testing it). Also, please contribute your thoughts testing results for my snippet and on the exception handling portability issues with throwing local objects from within a frame that is about to be unwound due to the throw. Thank you, V. ? ../../SimGear/source/simgear.doxytags ? ../../SimGear/source/simgear/misc/swap_test ? ../../SimGear/source/simgear/xgl/.deps ? ../../SimGear/source/simgear/xgl/Makefile ? ../../SimGear/source/simgear/xgl/Makefile.in Index: ../../SimGear/source/Doxyfile === RCS file: /var/cvs/SimGear-0.3/source/Doxyfile,v retrieving revision 1.15 diff -b -u -p -r1.15 Doxyfile --- ../../SimGear/source/Doxyfile 5 Nov 2005 19:30:52 - 1.15 +++ ../../SimGear/source/Doxyfile 25 Nov 2005 21:11:15 - @@ -46,17 +46,17 @@ OUTPUT_LANGUAGE= English # Private class members and static file members will be hidden unless # the EXTRACT_PRIVATE and EXTRACT_STATIC tags are set to YES -EXTRACT_ALL= NO +EXTRACT_ALL= YES # If the EXTRACT_PRIVATE tag is set to YES all private members of a class # will be included in the documentation. -EXTRACT_PRIVATE= NO +EXTRACT_PRIVATE= YES # If the EXTRACT_STATIC tag is set to YES all static members of a file # will be included in the documentation. -EXTRACT_STATIC = NO +EXTRACT_STATIC = YES # If the HIDE_UNDOC_MEMBERS tag is set to YES, Doxygen will hide all # undocumented members of documented classes, files or namespaces. @@ -111,7 +111,7 @@ STRIP_FROM_PATH= # to NO (the default) then the documentation will be excluded. # Set it to YES to include the internal documentation. -INTERNAL_DOCS = NO +INTERNAL_DOCS = YES # If the CLASS_DIAGRAMS tag is set to YES (the default) Doxygen will # generate a class diagram (in Html and LaTeX) for classes with base or @@ -498,7 +498,7 @@ TREEVIEW_WIDTH = 250 # If the GENERATE_LATEX tag is set to YES (the default) Doxygen will # generate Latex output. -GENERATE_LATEX = YES +GENERATE_LATEX = NO # The LATEX_OUTPUT tag is used to specify where the LaTeX docs will be put. # If a relative path is entered the value of OUTPUT_DIRECTORY will be @@ -558,7 +558,7 @@ LATEX_BATCHMODE= NO # The RTF output is
Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
> > > * whenever an exception object was created on a stack and then thrown
> > > (thus causing the dtor for that object to fire!), it was replaced
> > > with a STATIC exception
> >
> > The whole thing looks like a solution desperately searching for a
> > problem. The reasoning for this patch contradicts Stroustrup, who
> > has several examples of what we are doing in "The C++ programming
> > language". Maybe it's only because I'm using an older copy (2nd ed.),
> > but he writes (p. 602, "r.15.2 Throwing an Exception"):
> >
> > "A throw-expression initializes a temporary object of the static
> > type of the operand of throw and uses that temporary to initialize
> > the appropriately-typed variable named in the handler."
> >
> > The throw expression cares for the thrown class to be available
> > until it reached the handler. No need to spread ugly static
> > variables everywhere. Our code looks right for me as it is. But
I've created the following C++ snippet:
-- CUT HERE --
#include
#include
#include
using namespace std;
struct E : exception {
E() : _msg("DEFAULT") { cout << "E::E()" << endl; }
E(const char* s) : _msg(s) {
cout << "E::E(const char*" << s << ")" << endl; }
E(const E& e) : _msg(e._msg + "(clone)") {
cout << "E::E(const E&" << e._msg << ")" << endl; }
E& operator=(const E& e) {
cout << "E::operator=(" << e._msg << ") assigned over " << _msg <<
endl;
_msg = e._msg + "(assigned)";
return *this;
}
virtual ~E() throw() { cout << "E::~E() " << _msg << endl; }
const char* what() const throw() { _msg.c_str(); }
string _msg;
};
void foo(int i) throw(exception&)
{
cout << "foo entering" << endl;
E unused("xUNUSED");
switch (i) {
case 0:
break;
case 1:
throw E("x1");
case 2: {
E weird("x2");
throw weird;
}
}
cout << "foo exiting" << endl;
}
int main()
{
for (int i = 0; i < 3; i++) {
try {
foo(i);
}
catch (E& e) {
cout << "Caught " << e.what() << endl;
}
}
return 0;
}
-- CUT HERE --
and gcc C++ compiler version 3.3.5 (Debian 1:3.3.5-13) has this to
say when compiling it with default flags (fully confirming what
you say -- you can throw either tmp variables, or automatic ones,
created inside a frame to be unwound, and the objects won't be
destroyed until caught):
foo entering
E::E(const char*xUNUSED)
foo exiting
E::~E() xUNUSED
foo entering
E::E(const char*xUNUSED)
E::E(const char*x1)
E::~E() xUNUSED
Caught x1
E::~E() x1
foo entering
E::E(const char*xUNUSED)
E::E(const char*x2)
E::E(const E&x2)
E::~E() x2
E::~E() xUNUSED
Caught x2(clone)
E::~E() x2(clone)
If anybody has access to a suspicious compiler that's critical for their
platform's flightgear development, please do this test and check that
1) no exception is caught before its dtor is called
2) the amount of ctor calls equals the amount of dtor calls
3) no crash occurs
Vassilii
___
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d
Re: [Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
> * Vassilii Khachaturov -- Friday 25 November 2005 15:11: > > * whenever an exception object was created on a stack and then thrown > > (thus causing the dtor for that object to fire!), it was replaced > > with a STATIC exception > > The whole thing looks like a solution desperately searching for a > problem. The reasoning for this patch contradicts Stroustrup, who > has several examples of what we are doing in "The C++ programming > language". Maybe it's only because I'm using an older copy (2nd ed.), > but he writes (p. 602, "r.15.2 Throwing an Exception"): > > "A throw-expression initializes a temporary object of the static > type of the operand of throw and uses that temporary to initialize > the appropriately-typed variable named in the handler." > > The throw expression cares for the thrown class to be available > until it reached the handler. No need to spread ugly static > variables everywhere. Our code looks right for me as it is. But > then again, I'm a relative C++ newbie ... :-) AAARGH. If you had noticed me talking about the problem with JSB and others last 2 weeks, you'd have been able to prevent smth like 2 days of my work (patching and testing). :-) I didn't have a Stroustrup on hand, and several folks around did tell me this is a problem indeed. Thanks for the quote, better later than never. I wonder what compiler was JSB using in his string throwing example, can you please re-read that thread and see if you can find an alternative explanation? I'm sure that some (older) compilers do behave the ugly way I had described in my patch (this was definitely the case in the pre-ANSI-C++ days). If w/o this we're losing some platform (e.g., MSVC-based builds), maybe we should still apply it. (This is the problem of NOT being a C++ newbie --- you forget what's standard these days and what is coming from your habits to circumvent older quirks.) Nevertheless, a lot of things in the patch do make sense --- e.g., even in the light of what you quote, w/o proper copy semantics of the exception object, the things are broken. I can shred the conversion to static from the patch, but I'd like to hear more on whehter we do need the explicit static objects for some older compilers on other platforms. Vassilii ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
[Flightgear-devel] Re: [BUG] [PATCH] (announcement) throwing stale exceptions and missing copy ctor/assignment
* Vassilii Khachaturov -- Friday 25 November 2005 15:11: > * whenever an exception object was created on a stack and then thrown > (thus causing the dtor for that object to fire!), it was replaced > with a STATIC exception The whole thing looks like a solution desperately searching for a problem. The reasoning for this patch contradicts Stroustrup, who has several examples of what we are doing in "The C++ programming language". Maybe it's only because I'm using an older copy (2nd ed.), but he writes (p. 602, "r.15.2 Throwing an Exception"): "A throw-expression initializes a temporary object of the static type of the operand of throw and uses that temporary to initialize the appropriately-typed variable named in the handler." The throw expression cares for the thrown class to be available until it reached the handler. No need to spread ugly static variables everywhere. Our code looks right for me as it is. But then again, I'm a relative C++ newbie ... :-) m. ___ Flightgear-devel mailing list Flightgear-devel@flightgear.org http://mail.flightgear.org/mailman/listinfo/flightgear-devel 2f585eeea02e2c79d7b1d8c4963bae2d
