xserver: Cleaning up memory allocation functions and macros

Magnus Vigerlöf Magnus.Vigerlof at home.se
Sat Apr 28 09:37:15 PDT 2007


Hi all,

I've been checking the memory allocation functions (as requested) and would 
like to have some feedback on what I think should be changed.

Remember, this work will not affect the client side of things, only server and 
its drivers. I'll do my very best to retain the link-compability with the 
current source.


My proposal is the following:

* I haven't seen any place where INTERNAL_MALLOC is set/used (except in 
os/xalloc.c and os/utils.c) so let's remove every trace of that one. This 
also means that os/xalloc.c will be removed.

* Neither have I seen MEMBUG being defined anywhere. Will anyone miss this 
feature if I remove it? There are code that ignore the case where the memory 
allocation functions return NULL (the option functions (addNewOption2 [1]), I 
don't know how to make it better without breaking the interface though), so 
my gut feeling is that it's not used very much/at all today.

* Is there a reason for the existence of both xmalloc (macro) and Xmalloc 
(wrapper function)? (apply this question on all memory functions) I'll go for 
the libc-variants wherever I can, but those functions that depends on a 
slightly different semantic than the libc ones, which name standard should be 
used? (I prefer the lower case prefix [xstrdup, xnf...].)

* I'll go through the individual directories in xserver on-by-one and fix the 
code according to the 'standard' below once we agree on how to go forward.


So, what memory allocation functions should be used from now on?

* Use standard libc-functions
  - For every call to alloc/calloc/realloc/... there *must* be a check if it 
succeded to allocate the memory.
  - free() can take NULL as argument, so it's not needed to test for NULL 
before calling it.

* xstrdup() will be defined as a macro which can handle being passed NULL. 
strdup doesn't.

* xfreeZ() will be defined as a macro that set the pointer to NULL after 
freeing the memory with free(). Use *only* if the pointer must be set to NULL 
for some reason.

* xnf* will be implemented as wrapper functions that will call FatalError if 
the libc-functions returned NULL when they shouldn't (i.e. more or less kept 
as-is). NOTE: Avoid these, use proper error-propagation instead (calling 
these should be considered a BUG).


What have I missed?

Cheers
  Magnus V

[1] +196 hw/xfree86/parser/Flags.c



More information about the xorg mailing list