Re: [PATCH 3/3] xdiff: introduce XDL_ALLOC_GROW()

On Wed, Jun 29 2022, Phillip Wood via GitGitGadget wrote:

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.