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: OndÅej BÃlka <neleai at seznam dot cz>
- To: Paul Eggert <eggert at cs dot ucla dot edu>
- Cc: libc-alpha at sourceware dot org
- Date: Sun, 3 Nov 2013 21:24:22 +0100
- 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> <5276AB5B dot 8020208 at cs dot ucla dot edu>
On Sun, Nov 03, 2013 at 12:00:27PM -0800, Paul Eggert wrote:
> 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.
>
Tried but this gave warnings about unused function and that function
migth not be inlinable. I did not found how suppress these yet so macros
looked as better alternative.
> > + 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.
>
I will fix that.
> > + (__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.
>
When size is constant then check will be optimized away and this has no
overhead. I avoid preconditions that could be easily avoided without big
performance penalty.
> > #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.
I looked at XNMALLOC/xnmalloc code and how it is used. If you no not like
that I will remove these.