Page 1 of 1

Freeing NULL pointers...

Posted: 20 Feb 2008 23:08
by stevenh
To the devs,
I've just been playing around with the source in an archaic cross-compile environment and it turns out that my compiler/libraries do not like to free NULL pointers.
I've started a find/replace to replace any calls to free to my own safe_free which checks for a NULL pointer first.

Would this change be acceptable in the trunk? I would be angry if I had to start using safe_free instead of free.. but for this current port it is the easiest method to get around the current problem; unless someone knows how to inline a free() func that has the same name as the free() func that you then want to call :)

Thanks all.

Re: Freeing NULL pointers...

Posted: 20 Feb 2008 23:14
by peter1138
Hmm, even K&R's C book states that free(NULL) should just do nothing... Where do these implementors get their ideas from...

Re: Freeing NULL pointers...

Posted: 21 Feb 2008 00:23
by PhilSophus
stevenh wrote:unless someone knows how to inline a free() func that has the same name as the free() func that you then want to call :)
I guess your archaic environment doesn't support dynamic linking, does it? Otherwise, you could maybe provide your own implementation before the default libc (or however this is called in your environment) is loaded. E.g. on Linux this is used by memory leak checkers (LD_PRELOAD is the keyword here).

You may also just try to provide your own free implementation and hope that it does not try to take the one from the libraries then. However, I don't see how you could access the original implementation then. And writing your own memory management is probably nothing you would like to do :D

As an easy but dirty solution you could try this line in a header that is included anywhere you use free:

Code: Select all

#define free(x) safe_free(x)

Re: Freeing NULL pointers...

Posted: 21 Feb 2008 08:03
by peter1138
in your OS specific bit in stdafx.h or something...

Code: Select all

static inline safe_free(x)
{
   if (x != null) free(x);
}

#define free(x) safe_free(x)
maybe?

Re: Freeing NULL pointers...

Posted: 21 Feb 2008 08:16
by DaleStan
peter1138 wrote:Hmm, even K&R's C book states that free(NULL) should just do nothing.
Indeed. free(NULL);, delete NULL;, and delete[] NULL; are all no-ops.
Complain to whoever wrote your C library (or upgrade); it's non-conformant.

And while you're waiting for a fix for that, please go fix aRV refitting in Patch? It still only refits the first vehicle.

Re: Freeing NULL pointers...

Posted: 21 Feb 2008 08:20
by stevenh
I was just about to write "Isn't that classed as an infinite loop...." until I realised that the definition of the new 'free' is after the usage of the old 'free'....

Quite interesting... I might just do this... Thank all...

Now to work out endian issues... opntitle.dat wont work unless I have TO_LE32X in the Savegame formats which means that the 'direct' hdr[0] Uint32 that it's pulling out is being read in the "wrong" direction...

I have a mouse cursor showing though :D

Re: Freeing NULL pointers...

Posted: 21 Feb 2008 23:06
by prissi
About the free stuff: At least the Atari ST did not supported free(0), since the runtime (like many other too) give it to the OS free, which choked on it.

Aparently there are different interpretations of K&R delete NULL around too: I know of at least three compiler/development systems, where this would cause a warning or a breakpoint. (Albeit nothing that cannot be fixed by a define ... )

By the way, I think here is a confusion between K&R and ANSI. PDP-7 C (which was directly from K&R more or less) did not allowed free(NULL), it caused an error. At least it did when I did my first EDV courses.

Re: Freeing NULL pointers...

Posted: 22 Feb 2008 14:06
by egladil
Loading of opntitle.dat currently works fine both on big endian and little endian machines. Most likely the target endian was not detected correctly. You can use the --endian configure parameter to override this.

Re: Freeing NULL pointers...

Posted: 27 Feb 2008 02:18
by stevenh
A small update.... using peter1138's code worked until I attempted to flip endians and recompile everything... aystar.cpp uses a class function 'free' and seems to get this confused with the #define. This makes no sense to me, as the 'free' inside the class should not be overriden... for now I've renamed the class function.

Secondly... it seems that the libraries I am using for my system bring the CPU up as big-endian but the application that creates the romdisk writes the files as little endian... I reversed the readbyte functions in saveload.cpp, but things still seem a little weird... I'll screw around with it more once I get the time.

Re: Freeing NULL pointers...

Posted: 29 Feb 2008 11:38
by PhilSophus
stevenh wrote:A small update.... using peter1138's code worked until I attempted to flip endians and recompile everything... aystar.cpp uses a class function 'free' and seems to get this confused with the #define. This makes no sense to me, as the 'free' inside the class should not be overriden... for now I've renamed the class function.
That's why it is called a preprocessor. Preprocessor macros are substituted before the "real" compiling stage and thus ignore scoping completely and replace everything that matches. That is why you should avoid using preprocessor macros in C++ wherever possible (i.e. using inline functions or constants). Unfortunately, AFAICS this is one of the rare cases where there is no other way than using preprocessor macros. It's a hack anyway, so renaming the member function seems to be the best you can do about it.

Re: Freeing NULL pointers...

Posted: 29 Feb 2008 11:57
by Rainer
Hi,
PhilSophus wrote:
stevenh wrote:A small update.... using peter1138's code worked until I attempted to flip endians and recompile everything... aystar.cpp uses a class function 'free' and seems to get this confused with the #define. This makes no sense to me, as the 'free' inside the class should not be overriden... for now I've renamed the class function.
That's why it is called a preprocessor. Preprocessor macros are substituted before the "real" compiling stage and thus ignore scoping completely and replace everything that matches. That is why you should avoid using preprocessor macros in C++ wherever possible (i.e. using inline functions or constants). Unfortunately, AFAICS this is one of the rare cases where there is no other way than using preprocessor macros. It's a hack anyway, so renaming the member function seems to be the best you can do about it.
perhaps a little #undef will do it...

cu
Rainer

Re: Freeing NULL pointers...

Posted: 29 Feb 2008 12:01
by PhilSophus
Rainer wrote: perhaps a little #undef will do it...
That would soon going to get ugly as not only the definition of this free member is the problem. The calls to the member are problematic as well. You would have to be really careful where the #define should be in effect an where not. I think renaming is the cleaner solution (esp. easier to maintain).