Re: [PATCH] avoid pointer arithmetic involving NULL in FLEX_ALLOC_MEM

From: Jeff King <peff@peff.net>
Date: Oct 16, 2016, 19:46 UTC
Message-ID: <20161016194607.a6mhmdida57rhe23@sigill.intra.peff.net>
In-Reply-To: <c99474f2-dac3-e42a-5e4a-02464cac3982@web.de>

On Sun, Oct 16, 2016 at 12:06:02PM +0200, René Scharfe wrote:

Show 6 quoted lines

> > Yeah, this NULL computation is pretty nasty. I recall trying to get rid
> > of it, but I think it is impossible to do so portably while still using
> > the generic xalloc_flex() helper.
> 
> The only way I see is to pass the type to the macro explicitly (because
> typeof is an extention), and that would make call sites ugly.

Yep, exactly. I really wish we could use typeof(); there are a couple things that could be made less error-prone and ugly. But it's not even a matter of C99; it's a GNU-ism, so I don't think we are even close to having support for it everywhere.

Show 8 quoted lines

> > This looks correct. I wondered at first why you bothered with
> > flex_array_len, but it is to avoid evaluating the "len" parameter
> > multiple times.
> 
> Right; we could drop that feature of the original macros and require
> users to pass length expressions that don't have side effects -- all of
> them already do that anyway.  But let's keep it in this round; it just
> costs one extra line.

Oh, I think it's worth keeping forever, if only because it makes the macro interface have one fewer gotcha. Spending a line of code on that is OK with me.

Show 9 quoted lines

> > Now that xalloc_flex() has only this one caller remaining, perhaps it
> > should just be inlined here, too, for simplicity.
> 
> -- >8 --
> Subject: [PATCH 2/1] inline xalloc_flex() into FLEXPTR_ALLOC_MEM
> 
> Allocate and copy directly in FLEXPTR_ALLOC_MEM and remove the now
> unused helper function xalloc_flex().  The resulting code is shorter
> and the offset arithmetic is a bit simpler.

Looks good. Thanks for following up.

-Peff