]> Creatis software - gdcm.git/commitdiff
* python/distutilsSwigCPlusPlus.py now properly collects the
authorfrog <frog>
Wed, 22 Jan 2003 20:15:52 +0000 (20:15 +0000)
committerfrog <frog>
Wed, 22 Jan 2003 20:15:52 +0000 (20:15 +0000)
        shadow classes generated by "swig -c++" (gdcm.py in our case)
        when using "python setup.py install".
      * python/gdcmPython/__init__.py imports gdcm.py and only manually
        reexports the working classes.
      * src/gdcmHeader.cxx all the try/catch/throw calls were replaced
        by the classical C errno scheme. This is to prevent an obscure
        behavior of the python wrappers when importing wxPython.wx prior
        to gdcmPython (which ended up in an abort call). An illustration
        of this oddity can be found in the Test/ExceptionAndPython
        subdir (see the README file). This problem probably due to
        an combination of g++ and dynamic loading.
      * added Test/ExceptionAndPython (see above) --- Frog

12 files changed:
ChangeLog
Testing/ExceptionAndPython/.cvsignore [new file with mode: 0644]
Testing/ExceptionAndPython/Makefile [new file with mode: 0644]
Testing/ExceptionAndPython/README [new file with mode: 0644]
Testing/ExceptionAndPython/foo.cxx.minimal [new file with mode: 0644]
Testing/ExceptionAndPython/foo.cxx.orig [new file with mode: 0644]
Testing/ExceptionAndPython/foo.cxx.simpler [new file with mode: 0644]
Testing/ExceptionAndPython/foo.i [new file with mode: 0644]
Testing/ExceptionAndPython/foo_main.cxx.minimal [new file with mode: 0644]
Testing/ExceptionAndPython/foo_main.cxx.simpler [new file with mode: 0644]
Testing/ExceptionAndPython/test.py [new file with mode: 0644]
src/gdcmHeader.cxx

index b53b5f75721eca4b8981e3adb13997e3729d7c84..8f854425118b2dc7ecdf2c34ec14fa8b7773d771 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2003-01-17 Eric Boix <Eric.Boix@creatis.insa-lyon.fr>
+      * python/distutilsSwigCPlusPlus.py now properly collects the
+        shadow classes generated by "swig -c++" (gdcm.py in our case)
+        when using "python setup.py install".
+      * python/gdcmPython/__init__.py imports gdcm.py and only manually
+        reexports the working classes.
+      * src/gdcmHeader.cxx all the try/catch/throw calls were replaced
+        by the classical C errno scheme. This is to prevent an obscure
+        behavior of the python wrappers when importing wxPython.wx prior
+        to gdcmPython (which ended up in an abort call). An illustration
+        of this oddity can be found in the Test/ExceptionAndPython
+        subdir (see the README file). This problem probably due to
+        an combination of g++ and dynamic loading.
+      * added Test/ExceptionAndPython (see above)
+
 2003-01-17 Eric Boix <Eric.Boix@creatis.insa-lyon.fr>
       * Changed the layout of the python part to avoid bloating main
         directory with setup.py, distutilsSwigCPlusPlus.py, MANIFEST.in
@@ -10,7 +25,6 @@
       * gdcmPython/__init__.py doesn't export FileName any more (to avoid
         collision with other packages). gdcmPython/demo/*.py changed
         accordingly.
-
 2003-01-15 Eric Boix <Eric.Boix@creatis.insa-lyon.fr>
       * python subdir moved to gdcmPython (preparation of distutils packaging).
       * gdcmPython/setup.py and distutilsSwigCPlusPlus.py added. The
diff --git a/Testing/ExceptionAndPython/.cvsignore b/Testing/ExceptionAndPython/.cvsignore
new file mode 100644 (file)
index 0000000..d89fdcc
--- /dev/null
@@ -0,0 +1,6 @@
+foo.py
+foo_wrap.cxx
+foo.cxx
+main
+*.o
+*.so
diff --git a/Testing/ExceptionAndPython/Makefile b/Testing/ExceptionAndPython/Makefile
new file mode 100644 (file)
index 0000000..cf5982c
--- /dev/null
@@ -0,0 +1,13 @@
+all: main foo.so
+       
+
+main:
+       g++ -g -Wall -Wunused-variable  foo_main.cxx -o main
+
+foo.so:
+       g++ -c -g -Wall -Wunused-variable  foo.cxx -o foo.o
+       swig -python -c++  -o foo_wrap.cxx foo.i
+       g++ -c -g "-I`python -c "import sys; print sys.exec_prefix"`/include/python`python -c "import sys; print sys.version[:3]"`" foo_wrap.cxx -o foo_wrap.o
+       g++ -shared foo_wrap.o foo.o -o _foo.so -g
+clean:
+       rm -f *.o *.so foo.py foo.pyc foo_wrap.cxx main
diff --git a/Testing/ExceptionAndPython/README b/Testing/ExceptionAndPython/README
new file mode 100644 (file)
index 0000000..6e9fd69
--- /dev/null
@@ -0,0 +1,41 @@
+The purpous of this set of files is to illustrate an unexpected behavior
+of exception handling of swig wrapped c++ libraries with wxPython.
+Context:
+ - g++ version 2.96 or 3.2
+ - swig version 1.3.16u
+ - python2.2
+
+Consider the code in one of the foo_main.cxx say foo_main.cxx.minimal:
+       #include <iostream>
+       
+       void MyWrappedFunction(void) {
+          try { throw "In thrower"; }
+          catch (const char * str) { cout<<"Exception"<< str << endl; }
+       }
+       
+       int main() {
+               MyWrappedFunction();
+               return 0;
+       }
+When compiling this code and executing it one obtains the correct behavior
+of the exception i.e. ExceptionIn thrower.
+Now, wrap the above code with swig and invoque the following python
+script (see test.py):
+  import wxPython.wx
+  import foo
+  foo.MyWrappedFunction()
+Surprisingly enough the exception won't be caugth, but terminate will
+catch it and invoke abort.
+Note that the exception is properly caught when one doesn't import 
+wxPython.
+Note that the exception is properly caught when one doesn't import 
+wxPython.
+
+In order to play with this example, try the following:
+ln -s foo_main.cxx.minimal foo_main.cxx 
+ln -s foo.cxx.minimal foo.cxx 
+make
+./main
+ExceptionIn thrower
+python test.py
+
diff --git a/Testing/ExceptionAndPython/foo.cxx.minimal b/Testing/ExceptionAndPython/foo.cxx.minimal
new file mode 100644 (file)
index 0000000..7539b9e
--- /dev/null
@@ -0,0 +1,6 @@
+#include <iostream>
+
+void MyWrappedFunction(void) {
+   try { throw "In thrower"; }
+   catch (const char * str) { cout<<"Exception"<< str << endl; }
+}
diff --git a/Testing/ExceptionAndPython/foo.cxx.orig b/Testing/ExceptionAndPython/foo.cxx.orig
new file mode 100644 (file)
index 0000000..fbb8d58
--- /dev/null
@@ -0,0 +1,16 @@
+#include <iostream>
+
+namespace MyError {
+       struct MyException {
+               MyException() { cout << "In exception" << endl; }
+       };
+}
+
+void Thrower(void) {
+       throw MyError::MyException();
+}
+
+void MyWrappedFunction(void) {
+   try { Thrower(); }
+   catch (MyError::MyException) { cout<<"Exception caught in caller"<<endl; }
+}
diff --git a/Testing/ExceptionAndPython/foo.cxx.simpler b/Testing/ExceptionAndPython/foo.cxx.simpler
new file mode 100644 (file)
index 0000000..9112060
--- /dev/null
@@ -0,0 +1,15 @@
+#include <iostream>
+
+class MyException {
+public:
+       MyException() { cout << "In exception" << endl; }
+};
+
+void Thrower(void) {
+       throw MyException();
+}
+
+void MyWrappedFunction(void) {
+   try { Thrower(); }
+   catch (MyException &) { cout<<"Exception caught in caller"<<endl; }
+}
diff --git a/Testing/ExceptionAndPython/foo.i b/Testing/ExceptionAndPython/foo.i
new file mode 100644 (file)
index 0000000..22bf2ef
--- /dev/null
@@ -0,0 +1,5 @@
+%module foo
+%{
+void MyWrappedFunction(void);
+%}
+void MyWrappedFunction(void);
diff --git a/Testing/ExceptionAndPython/foo_main.cxx.minimal b/Testing/ExceptionAndPython/foo_main.cxx.minimal
new file mode 100644 (file)
index 0000000..aed3543
--- /dev/null
@@ -0,0 +1,11 @@
+#include <iostream>
+
+void MyWrappedFunction(void) {
+   try { throw "In thrower"; }
+   catch (const char * str) { cout<<"Exception"<< str << endl; }
+}
+
+int main() {
+       MyWrappedFunction();
+       return 0;
+}
diff --git a/Testing/ExceptionAndPython/foo_main.cxx.simpler b/Testing/ExceptionAndPython/foo_main.cxx.simpler
new file mode 100644 (file)
index 0000000..fe53517
--- /dev/null
@@ -0,0 +1,20 @@
+#include <iostream>
+
+struct MyException {
+   MyException() { std::cout << "In exception" << std::endl; }
+};
+
+void Thrower(void) {
+   throw MyException();
+}
+
+void MyWrappedFunction(void) {
+   try { Thrower(); }
+   catch (MyException){ std::cout<<"Exception caught in caller"<<std::endl; }
+}
+
+int main() {
+   MyWrappedFunction();
+  return 0;
+}
+
diff --git a/Testing/ExceptionAndPython/test.py b/Testing/ExceptionAndPython/test.py
new file mode 100644 (file)
index 0000000..70a3d48
--- /dev/null
@@ -0,0 +1,3 @@
+import wxPython.wx
+import foo
+foo.MyWrappedFunction()
index 6eeebe416a421f5327dd32076ca87adf9581ada7..dd8cdc09ee68b2768c3f491b06ffb4bba0816552 100644 (file)
@@ -2,6 +2,7 @@
 
 #include "gdcm.h"
 #include <stdio.h>
+#include <cerrno>
 // For nthos:
 #ifdef _MSC_VER
 #include <winsock.h>
 #define HEADER_LENGTH_TO_READ          256     // on ne lit plus que le debut
 #define _MaxSizeLoadElementValue_      1024    // longueur au dela de laquelle on ne charge plus les valeurs 
 
-namespace Error {
-       struct FileReadError {
-               FileReadError(FILE* fp, const char* Mesg) {
-                       if (feof(fp))
-                               dbg.Verbose(1, "EOF encountered :", Mesg);
-                       if (ferror(fp))
-                               dbg.Verbose(1, "Error on reading :", Mesg);
-               }
-       };
-}
-
 //FIXME: this looks dirty to me...
 
 #define str2num(str, typeNum) *((typeNum *)(str))
@@ -521,18 +511,22 @@ guint32 gdcmHeader::FindLengthOB(void) {
        while ( ! FoundSequenceDelimiter) {
                g = ReadInt16();
                n = ReadInt16();
+               if (errno == 1)
+                       return 0;
                TotalLength += 4;  // We even have to decount the group and element 
                if ( g != 0xfffe ) {
                        dbg.Verbose(1, "gdcmHeader::FindLengthOB: ",
                                    "wrong group for an item sequence.");
-                       throw Error::FileReadError(fp, "gdcmHeader::FindLengthOB");
+                       errno = 1;
+                       return 0;
                }
                if ( n == 0xe0dd )
                        FoundSequenceDelimiter = true;
                else if ( n != 0xe000) {
                        dbg.Verbose(1, "gdcmHeader::FindLengthOB: ",
                                    "wrong element for an item sequence.");
-                       throw Error::FileReadError(fp, "gdcmHeader::FindLengthOB");
+                       errno = 1;
+                       return 0;
                }
                ItemLength = ReadInt32();
                TotalLength += ItemLength + 4;  // We add 4 bytes since we just read
@@ -595,8 +589,11 @@ void gdcmHeader::FindLength(ElValue * ElVal) {
                // hands on a big endian encoded file: we switch the swap code to
                // big endian and proceed...
                if ( (element  == 0x000) && (length16 == 0x0400) ) {
-                       if ( ! IsExplicitVRBigEndianTransferSyntax() )
-                               throw Error::FileReadError(fp, "gdcmHeader::FindLength");
+                       if ( ! IsExplicitVRBigEndianTransferSyntax() ) {
+                               dbg.Verbose(0, "gdcmHeader::FindLength", "not explicit VR");
+                               errno = 1;
+                               return;
+                       }
                        length16 = 4;
                        SwitchSwapToBigEndian();
                        // Restore the unproperly loaded values i.e. the group, the element
@@ -783,11 +780,6 @@ void gdcmHeader::LoadElementValue(ElValue * ElVal) {
        }
        
        // FIXME The exact size should be length if we move to strings or whatever
-       
-       //
-       // QUESTION : y a-t-il une raison pour ne pas utiliser g_malloc ici ?
-       //
-       
        char* NewValue = (char*)malloc(length+1);
        if( !NewValue) {
                dbg.Verbose(1, "LoadElementValue: Failed to allocate NewValue");
@@ -798,7 +790,7 @@ void gdcmHeader::LoadElementValue(ElValue * ElVal) {
        item_read = fread(NewValue, (size_t)length, (size_t)1, fp);
        if ( item_read != 1 ) {
                free(NewValue);
-               Error::FileReadError(fp, "gdcmHeader::LoadElementValue");
+               dbg.Verbose(1, "gdcmHeader::LoadElementValue","unread element value");
                ElVal->SetValue("gdcm::UnRead");
                return;
        }
@@ -824,8 +816,12 @@ guint16 gdcmHeader::ReadInt16(void) {
        guint16 g;
        size_t item_read;
        item_read = fread (&g, (size_t)2,(size_t)1, fp);
-       if ( item_read != 1 )
-               throw Error::FileReadError(fp, "gdcmHeader::ReadInt16");
+       errno = 0;
+       if ( item_read != 1 ) {
+               dbg.Verbose(1, "gdcmHeader::ReadInt16", " File read error");
+               errno = 1;
+               return 0;
+       }
        g = SwapShort(g);
        return g;
 }
@@ -834,8 +830,12 @@ guint32 gdcmHeader::ReadInt32(void) {
        guint32 g;
        size_t item_read;
        item_read = fread (&g, (size_t)4,(size_t)1, fp);
-       if ( item_read != 1 )
-               throw Error::FileReadError(fp, "gdcmHeader::ReadInt32");
+       errno = 0;
+       if ( item_read != 1 ) {
+               dbg.Verbose(1, "gdcmHeader::ReadInt32", " File read error");
+               errno = 1;
+               return 0;
+       }
        g = SwapLong(g);
        return g;
 }
@@ -851,15 +851,12 @@ ElValue * gdcmHeader::ReadNextElement(void) {
        guint16 n;
        ElValue * NewElVal;
        
-       try {
-               g = ReadInt16();
-               n = ReadInt16();
-       }
-       catch ( Error::FileReadError ) {
+       g = ReadInt16();
+       n = ReadInt16();
+       if (errno == 1)
                // We reached the EOF (or an error occured) and header parsing
                // has to be considered as finished.
                return (ElValue *)0;
-       }
 
        // Find out if the tag we encountered is in the dictionaries:
        gdcmDictEntry * NewTag = IsInDicts(g, n);
@@ -873,10 +870,10 @@ ElValue * gdcmHeader::ReadNextElement(void) {
        }
 
        FindVR(NewElVal);
-       try { FindLength(NewElVal); }
-       catch ( Error::FileReadError ) { // Call it quits
+       FindLength(NewElVal);
+       if (errno == 1)
+               // Call it quits
                return (ElValue *)0;
-       }
        NewElVal->SetOffset(ftell(fp));
        return NewElVal;
 }