This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 1/2] Add nmalloc and nrealloc
- From: Paul Eggert <eggert at cs dot ucla dot edu>
- To: OndÅej BÃlka <neleai at seznam dot cz>
- Cc: libc-alpha at sourceware dot org
- Date: Sun, 03 Nov 2013 12:00:27 -0800
- Subject: Re: [PATCH v2 1/2] Add nmalloc and nrealloc
- Authentication-results: sourceware.org; auth=none
- References: <20131031164614 dot GA28117 at domone dot podge> <20131031202932 dot 8942074699 at topped-with-meat dot com> <20131103165411 dot GA27987 at domone dot podge>
OndÅej BÃlka wrote:
> I tried a inline function but gcc decided not to inline some of these.
Evidently you didn't use __attribute__ ((always_inline));
please try again, with that.
> + mem = nmalloc (2, ps);
This should be nmalloc (ps, 2). In general, if one argument is
a constant, it should be the element-size argument. There
are several other occurrences of this.
> + (__s == 0 || SIZE_MAX / __s < __n) ? NULL : malloc (__n * __s); \
The check for zero element size is not necessary. Similarly for nrealloc.
Just make it a precondition (i.e., the caller's responsibility)
that S must be nonzero. If you're worried about bugs in the caller,
you can add a compile-time check that the element-size argument is
a constant and is positive; I expect that all the current callers will
pass that check.
> #define NMALLOC(x, t) ((t *) nmalloc (x, sizeof (t)))
I would avoid macros whose arguments are types; in this
case they're not worth the extra confusion caused by the fact that
they're not function-like. Just have the callers invoke nmalloc,
with no NMALLOC macro. It's easier and less error-prone to
read and write code like this:
new_global = nmalloc (new_nalloc, sizeof *new_global);
than code like this:
new_global = NMALLOC (new_nalloc, struct link_map *);
particularly in cases, such as here, where the type is a
pointer.