On Wed, Jun 29 2022, Phillip Wood via GitGitGadget wrote:
> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
>
> Add a helper to grow an array. This is analogous to ALLOC_GROW() in
> the rest of the codebase but returns −1 on allocation failure to
> accommodate other users of libxdiff such as libgit2.
Urm, does it? I just skimmed this, so maybe I missed something, but I
don't see where you changed the definition of xdl_malloc(),
xdl_realloc() etc.
And those are defined as:
#define xdl_malloc(x) xmalloc(x)
#define xdl_free(ptr) free(ptr)
#define xdl_realloc(ptr,x) xrealloc(ptr,x)
And for e.g. xmalloc() we do:
return do_xmalloc(size, 0);
Where that 0=gently, i.e. we'll die() instead of error(), the xrealloc()
then has no "gently" option.
Is the (pretty glaring, if that's the case) unstated assumption here
that this doesn't in fact return -1 on allocation failure, but you're
expected to replace the underlying xmalloc() with an implementation that
does?
If so I'm doubly confused by this, since you're providing alternatives
to e.g.:
#define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))
So if that's the plan why would we need an XDL_ALLOC_ARRAY(), can't you
just check that it returns non-NULL?
That's not possible with our current xmalloc(), but ditto for this
series, and apparently the compiler isn't smart enough to yell at you
about that...
I wondered if we were just missing the returns_nonnull attribute, but
playing around with it I couldn't get GCC at least to warn about
checking xmalloc()'s return value for non-NULL.
