papadop
commented
8 years ago Cannot understand the travis failure. For me the test is skipped (as is the uncompressed case), so I do not understand why the test is not skipped there... Used valgrind to check for memory errors and found nothing.... I can understand that the patch could have introduced a uninitialized memory problem (forgot to say that some calloc where replaced by mallocs and proper memsets to initialize the memory where needed).
Is there a way to access the travis build ???
tbeu
agramfort
landscape-bot
This is another "just for discussion" pull request, even though it is already somewhat complete... In OpenMEEG, we had complains of "strange" error messages and crashes when allocating big matrices. The problem is that OpenMEEG being a C++ program, we rely on exceptions to capture errors and fail gracefully. But matio uses plain malloc and justs exits (sometimes) when a memory allocation failure arises. In this branch, I replaced all memory allocations by macros that expand (or are equivalent to) the C code when compiled with C and to C++ allocation routines when compiling with C++. This is tested to work fine in both C and C++ compilations.... So now, if compiling in C++ mode, allocation failure now result in exceptions emitted.
The drawback is that in C++ mode, no cleanup is made (because the if (var==NULL) blocks are not executed. To deal with that, I need to uglify the code with macros simulating TRY {} CATCH() {}, which will become try {} catch(...) { CLEANUP } blocks in C++ mode and traditional if (var==NULL) {CLEANUP} blocks in C mode.
One nice side effect of this work is that it is easy to identify all allocation points by grepping NEW. And if the TRY {}CATCH {} work is done, then all memory allocation failures will be caught, whereas this seems not to be the case currently.
Among other C++ improvements I might be interested to create are
Of course the first and second points need to be done without breaking the C interface. I have ideas, but I'm not totally sure if this can be done without uglifying the code too much. The last point is fairly easy (at least with the cmake build system I use, which will be the subject of another "just for discussion" pull request I will make once all the test reorganisation stuff will be integrated. With autoconf, this is doable but may need two separate builds (as it works currently, but we need to detect the C++ compiler and change the final library name).