xserver: Cleaning up memory allocation functions and macros

[Daniel Stone]

On Sat, Apr 28, 2007 at 06:37:15PM +0200, Magnus Vigerlöf wrote:
> * 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.

Right.  It has never been used in the modular tree.

> * 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.

Not really.  I guess the best way of testing this would be to get a libc
modification in which did the same.

> * 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...].)

Probably to do with ABI concerns from 1993.

> * 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.

Huzzah!  Thankyou.

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

Sounds sensible, but you could also flag a warning if --enable-debug is
enabled, and we could fix all these cases and gradually transition to
using normal strdup()?

> * 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.

Is there any reason to do this?  It just encourages the x*() wrappers
and makes the code less obvious to the casual observer.

> * 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).

Indeed.  You can just mark these as _X_DEPRECATED and the compiler will
complain every time.

Thanks for picking these up.

Cheers,
Daniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg/attachments/20070428/e3afe444/attachment.pgp>