This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] malloca v2
Some comments:
On 01/13/2013 02:05 AM, OndÅej BÃlka wrote:
> When memory cannot be allocated from heap
> malloca aborts.
Isn't it common for functions to want to fail (setting errno ==
ENOMEM, say) when memory cannot be allocated? These functions can't
call malloca if it has the described behavior. Shouldn't malloca, or
a variant, support such functions?
> You must write MALLOCA_INIT() at start of each function that uses malloca.
> Memory is freed automaticaly.
Since MALLOCA_INIT is not a function, and doesn't act at all like a
function, I suggest that it be a plain macro MALLOCA_INIT and not a
function-like macro MALLOC_INIT ().
The documentation should explain more carefully how the memory is
allocated and freed, and any restrictions on it. Is it OK to put
declarations before MALLOCA_INIT? Is it OK to put MALLOCA_INIT inside
a nested block? If the latter, does the storage allocated by malloca
have a lifetime equal to that of the calling function, or of the
containing block? If the calling function is inlined, does the
allocated storage persist into the calling function's caller? Can one
use malloca and variable-length arrays in the same function, and if
so, how do they interact? That sort of thing.
> /* Safe automatic memory allocation.
> Copyright (C) 2012 Free Software Foundation, Inc.
Should be '2012-2013'.
> void* __malloca_alloc (void *__malloca_gc,size_t __n);
Please use glibc style: "void *x" rather than "void* x" or "void * x",
and pout a space after the comma. There were several problems of
this sort.
> static inline void __ifreeall (void * __malloca_gc)
> {
Similarly, don't indent the "{", and put __ifreeall on a new line.
> void * __tmp = __malloca_gc;
> __malloca_gc = ((void **)__malloca_gc)[0];
The following is simpler, and avoids a cast:
void **__tmp = __malloca_gc;
__malloca_gc = *tmp;
> void *__r = malloc (__n + _MALLOC_ALIGNMENT);
> if (!__r || __n+_MALLOC_ALIGNMENT < _MALLOC_ALIGNMENT)
> abort ();
> ((void **)__r)[0] = __malloca_gc;
This assumes that size_t is at least as wide as int -- does glibc
guarantee that? And it's better to avoid casts. Something like the
following, perhaps.
size_t __n1 = __n + _MALLOC_ALIGNMENT;
void *__r = malloc (__n1);
void **__pr = __r;
if (!__r || n1 < _MALLOC_ALIGNMENT)
abort ();
*__pr = __malloca_gc;
> #define malloca(n) ({ \
> size_t __n = (n); \
> void * __r = NULL; \
No need to initialize __r there.
> #undef alloca_account
> #define alloca_account(n, var) ({ \
> void *__r = alloca(n); \
> __r; \
> })
Why is this needed? A comment perhaps.