Menu
▾
▴
#70 Remove unnecessary null pointer checks
The null pointer check for the variable "arg2" is not needed for the delete instruction in functions like "_wrap_SbTime___sub__" and "_wrap_SbString___iadd____SWIG_1" and the variable "self" in functions like "delete_floatp" and "delete_longp".
http://dietmar-kuehl.de/mirror/c++-faq/freestore-mgmt.html#faq-16.8
http://free-cad.svn.sourceforge.net/viewvc/free-cad/trunk/src/3rdParty/Pivy/soqt_wrap.cpp?revision=1373&view=markup
You've given next to no information about what you want done. What language are you talking about? What is the source code for the problem? What are the command line options you are using?
What you have provide is like providing a tiny snippet of machine code that a compiler has generated and saying this needs improving without giving the C source.
My guess is Markus is trying to point out that in a lot of typemaps SWIG does:
if (ptr) delete ptr;
or:
if (ptr) free(ptr);
Even if that wasn't what he meant, this is a valid issue. In C++, delete NULL is explicitly a no-op, and similar for free() in ISO C. Pre-ISO C libraries didn't always allow free(NULL); but I suspect that's a non-issue now - ANSI C was 1989, which is 21 years ago. So these checks just add needless overhead. The runtime overhead is probably in the noise (and I wouldn't be totally surprised if some compilers know they can just optimise away such checks) but SWIG wrappers for larger APIs can take a lot of memory to compile, and removing these checks should help reduce that a little.
I have made a start on a patch for this - I'll try to finish it off and attach it here for consideration.
Your are right. I suggest such a clean-up for null pointer checks.
Will an ISO C/C++ compiler be a requirement for your current software development?
Well, my view would be that insisting on total ISO compliance from the compiler in every detail is a bit hard line - if a popular compiler differs in a way which we can work around, I think we should.
But we have a small number of developers with a limited amount of time they can spend working on SWIG, and there are better ways to spend that time than trying to support kit which now realistically only exists in museums or enthusiasts' basements, plus perhaps the odd deployed machine which has been in place so long that nobody dare touch it anyway.
Others might have different views, which is why I said I'd attach the patch here for comment.
Sorry, seems I never followed through on that patch. I'll see if I can find it.
Found the WIP patch, and did another pass to find some more cases.
PR opened: https://github.com/swig/swig/pull/2184
I'll merge when CI passes - closing this now so I don't forget as there's no automatic closing.
Thanks for your source code improvement.