This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Use malloca instead alloca
Hi!
I applaud any effort that absolves us of the current unholy pointer
flags we need to keep track of now.
On Sat, Dec 29, 2012 at 11:33:15AM +0100, Ondřej Bílka wrote:
> /* Safe automatic memory allocation.
> Copyright (C) 2012 Free Software Foundation, Inc.
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2, or (at your option)
> any later version.
>
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU General Public License for more details.
>
> You should have received a copy of the GNU General Public License
> along with this program; if not, see <http://www.gnu.org/licenses/>. */
>
> #ifndef _MALLOCA_H
> #define _MALLOCA_H
>
> /* AIX requires this to be the first thing in the file. */
> #ifndef __GNUC__
> # if HAVE_ALLOCA_H || defined _LIBC
> # include <alloca.h>
> # else
> # ifdef _AIX
> #pragma alloca
> # else
> # ifndef alloca /* predefined by HP cc +Olibcalls */
> char *alloca ();
> # endif
Does this matter if we go ahead with using ({...}) just a few lines
below?
> # endif
> # endif
> #endif
>
> #include <stddef.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <stdint.h>
>
> #ifdef __cplusplus
> extern "C" {
> #endif
>
> #define _ALLOCA_MC 0x2439a2431bca4812L
> #define _MALLOC_MC 0x1bca48122439a243L
Does the 'L' have any effect? I'm not sure if it isn't actually doing
any harm on 32-bit platforms... If anything, I'd use 'ULL', but maybe no
suffix should be necessary.
I assume uint64_t is used to preserve alignment? This question doesn't
appear as trivial to me, as there are some significant considerations
behind MALLOC_ALIGNMENT; malloca() returns less aligned pointers than
malloc() on 64-bit platforms, which maybe matters just for exotic cases
like long double which probably won't be an issue in glibc, but this
should be documented somewhere.
> /* malloca(N) is a safe variant of alloca(N). It allocates N bytes of
> memory allocated on the stack or heap for large requests.
> It must be freed using freea() before
> the function returns. Upon failure, it returns NULL. */
>
> #if 1
#if 1?
> #define malloca(n) ({\
> size_t __n__ = n;\
> void * __r__ = NULL;\
> if (__n__ <= 4096)\
> {\
> __r__ = alloca (__n__ + sizeof (uint64_t));\
> if (__r__)\
> {\
> *((uint64_t *)__r__) = _ALLOCA_MC;\
> __r__ += sizeof (uint64_t);\
> }\
> }\
> if (!__r__)\
> {\
> __r__ = malloc (__n__ + sizeof (uint64_t));\
> if (__r__)\
> {\
> *((uint64_t *)__r__) = _MALLOC_MC;\
> __r__ += sizeof (uint64_t);\
> }\
> }\
> __r__;\
> })
So it is still unsafe to call malloca() in a loop. This is also
something that should be documented. Or can't we do better? I think even
having an alternative 2-parameter call would be worth considering so
that users that need this can use a local variable to keep track of
stack usage, as per include/alloca.h.
> /* If desired we could detect more corruption by
> adding constant to end of alloca'd array. */
>
> #define freea(r) {\
> void *__r__ = r;\
> if (__r__)\
> {\
> __r__ -= sizeof (uint64_t);\
> if ( *((uint64_t *)__r__) == _MALLOC_MC)\
> free (__r__);\
> else if (*((uint64_t *)__r__) != _ALLOCA_MC)\
> __abort_freea();\
> }\
> }
I personally find it much more readable to have " \" instead of "\" at
line ends; maybe GNU style even requires having the \s aligned at col 78.
You really should wrap the freea() block in do { ... } while (0)
Petr "Pasky" Baudis