Re: [PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate
On Fri, Nov 09, 2018 at 06:21:41PM +0100, Johannes Schindelin wrote:
Show 11 quoted lines
> Actually, you got me thinking about the desc.buffer. And I think there is
> one corner case where it could cause a problem: `struct tree_desc desc[2]`
> does not initialize the buffers to NULL. And what if
> fill_tree_descriptor() function returns NULL? Then the buffer is still
> uninitialized.
>
> In practice, our *current* version of fill_tree_descriptor() never returns
> NULL if the oid parameter is non-NULL. It would die() in the error case
> instead (bad!). So to prepare for a less bad version, I'd rather
> initialize the `desc` array and be on the safe (and easier to reason
> about) side.
Yeah, I agree with all of that.
One solution would just be to increment only after success:
if (fill_tree_descriptor(&desc[nr], ..) < 0)
goto error;
nr++; /* now we know it's valid! */
But there are lots of alternatives. :)
-Peff
Jeff King <peff@peff.net>