From 5473319b31bf630394e7865122f1745ecc122a4b Mon Sep 17 00:00:00 2001 From: frog Date: Wed, 22 Jan 2003 20:15:52 +0000 Subject: [PATCH] * 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) --- Frog --- ChangeLog | 16 ++++- Testing/ExceptionAndPython/.cvsignore | 6 ++ Testing/ExceptionAndPython/Makefile | 13 ++++ Testing/ExceptionAndPython/README | 41 ++++++++++++ Testing/ExceptionAndPython/foo.cxx.minimal | 6 ++ Testing/ExceptionAndPython/foo.cxx.orig | 16 +++++ Testing/ExceptionAndPython/foo.cxx.simpler | 15 +++++ Testing/ExceptionAndPython/foo.i | 5 ++ .../ExceptionAndPython/foo_main.cxx.minimal | 11 ++++ .../ExceptionAndPython/foo_main.cxx.simpler | 20 ++++++ Testing/ExceptionAndPython/test.py | 3 + src/gdcmHeader.cxx | 65 +++++++++---------- 12 files changed, 182 insertions(+), 35 deletions(-) create mode 100644 Testing/ExceptionAndPython/.cvsignore create mode 100644 Testing/ExceptionAndPython/Makefile create mode 100644 Testing/ExceptionAndPython/README create mode 100644 Testing/ExceptionAndPython/foo.cxx.minimal create mode 100644 Testing/ExceptionAndPython/foo.cxx.orig create mode 100644 Testing/ExceptionAndPython/foo.cxx.simpler create mode 100644 Testing/ExceptionAndPython/foo.i create mode 100644 Testing/ExceptionAndPython/foo_main.cxx.minimal create mode 100644 Testing/ExceptionAndPython/foo_main.cxx.simpler create mode 100644 Testing/ExceptionAndPython/test.py diff --git a/ChangeLog b/ChangeLog index b53b5f75..8f854425 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2003-01-17 Eric Boix + * 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 * 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 * 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 index 00000000..d89fdcc7 --- /dev/null +++ b/Testing/ExceptionAndPython/.cvsignore @@ -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 index 00000000..cf5982c3 --- /dev/null +++ b/Testing/ExceptionAndPython/Makefile @@ -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 index 00000000..6e9fd69e --- /dev/null +++ b/Testing/ExceptionAndPython/README @@ -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 + + 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 index 00000000..7539b9ec --- /dev/null +++ b/Testing/ExceptionAndPython/foo.cxx.minimal @@ -0,0 +1,6 @@ +#include + +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 index 00000000..fbb8d581 --- /dev/null +++ b/Testing/ExceptionAndPython/foo.cxx.orig @@ -0,0 +1,16 @@ +#include + +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"< + +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"< + +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 index 00000000..fe535175 --- /dev/null +++ b/Testing/ExceptionAndPython/foo_main.cxx.simpler @@ -0,0 +1,20 @@ +#include + +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"< +#include // For nthos: #ifdef _MSC_VER #include @@ -16,17 +17,6 @@ #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; } -- 2.48.1