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 -0000       1.15
+++ ../../SimGear/source/Doxyfile       25 Nov 2005 21:11:15 -0000
@@ -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 optimised for Word 97 and may not look very pretty with 
 # other RTF readers or editors.
 
-GENERATE_RTF           = YES
+GENERATE_RTF           = NO
 
 # The RTF_OUTPUT tag is used to specify where the RTF docs will be put. 
 # If a relative path is entered the value of OUTPUT_DIRECTORY will be 
@@ -674,7 +674,7 @@ TAGFILES               = 
 # When a file name is specified after GENERATE_TAGFILE, doxygen will create 
 # a tag file that is based on the input files it reads. 
 
-GENERATE_TAGFILE       = 
+GENERATE_TAGFILE       = simgear.doxytags
 
 # If the ALLEXTERNALS tag is set to YES all external classes will be listed 
 # in the class index. If set to NO only the inherited external classes 
@@ -696,7 +696,7 @@ PERL_PATH              = /usr/bin/perl
 # toolkit from AT&T and Lucent Bell Labs. The other options in this section 
 # have no effect if this option is set to NO (the default) 
 
-HAVE_DOT               = NO
+HAVE_DOT               = YES
 
 # If the CLASS_GRAPH and HAVE_DOT tags are set to YES then doxygen 
 # will generate a graph for each documented class showing the direct and 
Index: ../../SimGear/source/simgear/environment/metar.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/source/simgear/environment/metar.cxx,v
retrieving revision 1.7
diff -b -u -p -r1.7 metar.cxx
--- ../../SimGear/source/simgear/environment/metar.cxx  6 Oct 2005 09:45:36 
-0000       1.7
+++ ../../SimGear/source/simgear/environment/metar.cxx  25 Nov 2005 21:11:16 
-0000
@@ -107,7 +107,7 @@ SGMetar::SGMetar(const string& m, const 
        scanType();
        if (!scanId() || !scanDate()) {
                delete[] _data;
-               throw sg_io_exception("metar data bogus (" + _url + ')');
+               throw sg_io_exception("metar data bogus ", sg_location(_url));
        }
        scanModifier();
 
@@ -133,7 +133,7 @@ SGMetar::SGMetar(const string& m, const 
 
        if (_grpcount < 4) {
                delete[] _data;
-               throw sg_io_exception("metar data incomplete (" + _url + ')');
+               throw sg_io_exception("metar data incomplete ", 
sg_location(_url));
        }
 
        _url = "";
@@ -196,7 +196,7 @@ char *SGMetar::loadData(const char *id, 
        sock->set_timeout(10000);
        if (!sock->open(SG_IO_OUT)) {
                delete sock;
-               throw sg_io_exception("cannot connect to " + host);
+               throw sg_io_exception("cannot connect to ", sg_location(host));
        }
 
        string get = "GET ";
@@ -233,7 +233,8 @@ char *SGMetar::loadData(const char *id, 
        char *b = buf;
        scanBoundary(&b);
        if (*b == '<')
-               throw sg_io_exception("no metar data available from " + _url);
+               throw sg_io_exception("no metar data available from ", 
+                               sg_location(_url));
 
        char *metar = new char[strlen(b) + 2];  // make room for " \0"
        strcpy(metar, b);
Index: ../../SimGear/source/simgear/scene/material/matlib.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/source/simgear/scene/material/matlib.cxx,v
retrieving revision 1.16
diff -b -u -p -r1.16 matlib.cxx
--- ../../SimGear/source/simgear/scene/material/matlib.cxx      23 Oct 2005 
13:47:46 -0000      1.16
+++ ../../SimGear/source/simgear/scene/material/matlib.cxx      25 Nov 2005 
21:11:19 -0000
@@ -185,7 +185,7 @@ bool SGMaterialLib::load( const string &
     } catch (const sg_exception &ex) {
         SG_LOG( SG_INPUT, SG_ALERT, "Error reading materials: "
                 << ex.getMessage() );
-        throw ex;
+        throw;
     }
 
     int nMaterials = materials.nChildren();
Index: ../../SimGear/source/simgear/scene/model/model.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/source/simgear/scene/model/model.cxx,v
retrieving revision 1.35
diff -b -u -p -r1.35 model.cxx
--- ../../SimGear/source/simgear/scene/model/model.cxx  25 Sep 2005 07:44:50 
-0000      1.35
+++ ../../SimGear/source/simgear/scene/model/model.cxx  25 Nov 2005 21:11:20 
-0000
@@ -283,7 +283,8 @@ sgLoad3DModel( const string &fg_root, co
     ssgTexturePath((char *)texturepath.c_str());
     model = (ssgBranch *)ssgLoad((char *)modelpath.c_str());
     if (model == 0)
-      throw sg_exception("Failed to load 3D model");
+      throw sg_io_exception("Failed to load 3D model", 
+                         sg_location(modelpath.str()));
   }
                                 // Set up the alignment node
   ssgTransform * alignmainmodel = new ssgTransform;
Index: ../../SimGear/source/simgear/sound/sample_openal.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/source/simgear/sound/sample_openal.cxx,v
retrieving revision 1.23
diff -b -u -p -r1.23 sample_openal.cxx
--- ../../SimGear/source/simgear/sound/sample_openal.cxx        12 Nov 2005 
12:22:23 -0000      1.23
+++ ../../SimGear/source/simgear/sound/sample_openal.cxx        25 Nov 2005 
21:11:21 -0000
@@ -118,7 +118,8 @@ SGSoundSample::SGSoundSample( const char
   if (buffer == AL_NONE) {
      ALenum error = alutGetError ();
      print_openal_error("constructor (alutCreateBufferFromFile)");
-     throw sg_exception("Failed to load wav file: "+string(alutGetErrorString 
(error)));
+     throw sg_io_exception("Failed to load wav file: ",
+                        sg_location(string(alutGetErrorString (error))));
   }
 
 #else
@@ -383,7 +384,8 @@ SGSoundSample::load_file(const char *pat
     ALfloat freqf;
     data = alutLoadMemoryFromFile(samplepath.c_str(), &format, &size, &freqf );
     if (data == NULL) {
-        throw sg_exception("Failed to load wav file.");
+        throw sg_io_exception("Failed to load wav file.",
+                                       sg_location(samplepath.str()));
     }
     freq = (ALsizei)freqf;
 #else
@@ -395,7 +397,8 @@ SGSoundSample::load_file(const char *pat
                      &format, &data, &size, &freq, &loop );
 # endif
     if ( print_openal_error("constructor (alutLoadWAVFile)") ) {
-        throw sg_exception("Failed to load wav file.");
+        throw sg_io_exception("Failed to load wav file.",
+                                       sg_location(samplepath.str()));
     }
 #endif
 
Index: ../../SimGear/source/simgear/structure/exception.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/source/simgear/structure/exception.cxx,v
retrieving revision 1.3
diff -b -u -p -r1.3 exception.cxx
--- ../../SimGear/source/simgear/structure/exception.cxx        19 Dec 2003 
17:44:30 -0000      1.3
+++ ../../SimGear/source/simgear/structure/exception.cxx        25 Nov 2005 
21:11:21 -0000
@@ -21,6 +21,23 @@ sg_location::sg_location ()
 {
 }
 
+sg_location::sg_location (const sg_location& other)
+  : _path(other._path),
+    _line(other._line),
+    _column(other._column),
+    _byte(other._byte)
+{
+}
+
+sg_location& sg_location::operator= (const sg_location& other)
+{
+       _path = other._path;
+       _line = other._line;
+       _column = other._column;
+       _byte = other._byte;
+       return *this;
+}
+
 sg_location::sg_location (const string &path, int line, int column)
   : _path(path),
     _line(line),
@@ -123,6 +140,19 @@ sg_throwable::sg_throwable (const string
 {
 }
 
+sg_throwable::sg_throwable (const sg_throwable& other)
+  : _message(other._message),
+    _origin(other._origin)
+{
+}
+
+sg_throwable& sg_throwable::operator= (const sg_throwable& other)
+{
+         _message = other._message;
+         _origin = other._origin;
+         return *this;
+}
+
 sg_throwable::~sg_throwable ()
 {
 }
@@ -193,6 +223,17 @@ sg_exception::sg_exception (const string
 {
 }
 
+sg_exception::sg_exception (const sg_exception& other)
+  : sg_throwable(other)
+{
+}
+
+sg_exception& sg_exception::operator= (const sg_exception& other)
+{
+       sg_throwable::operator=(other);
+       return *this;
+}
+
 sg_exception::~sg_exception ()
 {
 }
@@ -221,6 +262,19 @@ sg_io_exception::sg_io_exception (const 
 {
 }
 
+sg_io_exception::sg_io_exception (const sg_io_exception& other)
+  : sg_exception(other),
+       _location(other._location)
+{
+}
+
+sg_io_exception& sg_io_exception::operator= (const sg_io_exception& other)
+{
+       sg_exception::operator=(other);
+       _location = other._location;
+       return *this;
+}
+
 sg_io_exception::~sg_io_exception ()
 {
 }
Index: ../../SimGear/source/simgear/structure/exception.hxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/source/simgear/structure/exception.hxx,v
retrieving revision 1.1
diff -b -u -p -r1.1 exception.hxx
--- ../../SimGear/source/simgear/structure/exception.hxx        24 Sep 2003 
17:19:23 -0000      1.1
+++ ../../SimGear/source/simgear/structure/exception.hxx        25 Nov 2005 
21:11:22 -0000
@@ -28,6 +28,8 @@ class sg_location
 public:
   sg_location ();
   sg_location (const string &path, int line = -1, int column = -1);
+  sg_location (const sg_location& other);
+  sg_location& operator=(const sg_location& other);
   virtual ~sg_location ();
   virtual const string &getPath () const;
   virtual void setPath (const string &path);
@@ -47,13 +49,15 @@ private:
 
 
 /**
- * Abstract base class for all throwables.
+ * Base class for all throwables.
  */
 class sg_throwable
 {
 public:
   sg_throwable ();
   sg_throwable (const string &message, const string &origin = "");
+  sg_throwable (const sg_throwable& other);
+  sg_throwable& operator= (const sg_throwable& other);
   virtual ~sg_throwable ();
   virtual const string &getMessage () const;
   virtual const string getFormattedMessage () const;
@@ -104,6 +108,8 @@ class sg_exception : public sg_throwable
 public:
   sg_exception ();
   sg_exception (const string &message, const string &origin = "");
+  sg_exception (const sg_exception& other);
+  sg_exception& operator= (const sg_exception& other);
   virtual ~sg_exception ();
 };
 
@@ -123,6 +129,8 @@ class sg_io_exception : public sg_except
 {
 public:
   sg_io_exception ();
+  sg_io_exception (const sg_io_exception& other);
+  sg_io_exception& operator= (const sg_io_exception& other);
   sg_io_exception (const string &message, const string &origin = "");
   sg_io_exception (const string &message, const sg_location &location,
                   const string &origin = "");
Index: ../../SimGear/source/simgear/xml/easyxml.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/source/simgear/xml/easyxml.cxx,v
retrieving revision 1.6
diff -b -u -p -r1.6 easyxml.cxx
--- ../../SimGear/source/simgear/xml/easyxml.cxx        15 Sep 2005 16:54:27 
-0000      1.6
+++ ../../SimGear/source/simgear/xml/easyxml.cxx        25 Nov 2005 21:11:22 
-0000
@@ -263,12 +263,12 @@ readXML (const string &path, XMLVisitor 
   if (input.good()) {
     try {
       readXML(input, visitor, path);
-    } catch (sg_io_exception &e) {
+    } catch (sg_io_exception &) {
       input.close();
-      throw e;
-    } catch (sg_throwable &t) {
+      throw;
+    } catch (sg_throwable &) {
       input.close();
-      throw t;
+      throw;
     }
   } else {
     throw sg_io_exception("Failed to open file", sg_location(path),
Index: ../../FlightGear/source/src/ATC/AIMgr.cxx
===================================================================
RCS file: /var/cvs/FlightGear-0.9/source/src/ATC/AIMgr.cxx,v
retrieving revision 1.29
diff -b -u -p -r1.29 AIMgr.cxx
--- ../../FlightGear/source/src/ATC/AIMgr.cxx   11 Nov 2005 13:45:35 -0000      
1.29
+++ ../../FlightGear/source/src/ATC/AIMgr.cxx   25 Nov 2005 13:21:55 -0000
@@ -82,7 +82,7 @@ void FGAIMgr::init() {
                                           planepath.c_str(),
                                                                           
globals->get_props(),
                                                                           
globals->get_sim_time_sec() );
-       } catch(sg_exception& e) {
+       } catch(sg_exception&) {
                _loadedDefaultOK = false;
        }
        
@@ -102,7 +102,7 @@ void FGAIMgr::init() {
                                         planepath.c_str(),
                                                                         
globals->get_props(),
                                                                         
globals->get_sim_time_sec() );
-       } catch(sg_exception& e) {
+       } catch(sg_exception&) {
                _havePiperModel = false;
        }
 
Index: ../../FlightGear/source/src/Main/options.cxx
===================================================================
RCS file: /var/cvs/FlightGear-0.9/source/src/Main/options.cxx,v
retrieving revision 1.76
diff -b -u -p -r1.76 options.cxx
--- ../../FlightGear/source/src/Main/options.cxx        13 Nov 2005 10:20:15 
-0000      1.76
+++ ../../FlightGear/source/src/Main/options.cxx        25 Nov 2005 13:22:16 
-0000
@@ -1063,7 +1063,7 @@ fgOptConfig( const char *arg )
        readProperties(file, globals->get_props());
     } catch (const sg_exception &e) {
        string message = "Error loading config file: ";
-       message += e.getFormattedMessage();
+       message += e.getFormattedMessage() + e.getOrigin();
        SG_LOG(SG_INPUT, SG_ALERT, message);
        exit(2);
     }
Index: ../../FlightGear/source/src/Scenery/tilemgr.cxx
===================================================================
RCS file: /var/cvs/FlightGear-0.9/source/src/Scenery/tilemgr.cxx,v
retrieving revision 1.50
diff -b -u -p -r1.50 tilemgr.cxx
--- ../../FlightGear/source/src/Scenery/tilemgr.cxx     22 Nov 2005 17:02:31 
-0000      1.50
+++ ../../FlightGear/source/src/Scenery/tilemgr.cxx     25 Nov 2005 13:22:17 
-0000
@@ -323,8 +323,13 @@ void FGTileMgr::update_queues()
                             SGShadowVolume::occluderTypeTileObject,
                             (ssgBranch *) 
dm->get_tile()->get_terra_transform());
                     }
-                } catch (const sg_exception& exc) {
-                    SG_LOG( SG_ALL, SG_ALERT, exc.getMessage() );
+                } catch (const sg_io_exception& exc) {
+                                       string m(exc.getMessage());
+                                       m += " ";
+                                       m += exc.getLocation().asString();
+                    SG_LOG( SG_ALL, SG_ALERT, m );
+                } catch (const sg_exception& exc) { // XXX may be redundant
+                    SG_LOG( SG_ALL, SG_ALERT, exc.getMessage());
                 }
                 
                 dm->get_tile()->dec_pending_models();
_______________________________________________
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d

Reply via email to