From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 12 Nov 2018 11:11:36 GMT
Subject: Re: [PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate
Message-ID: <nycvar.QRO.7.76.6.1811121211040.39@tvgsbejvaqbjf.bet>
In-Reply-To: <20181109195934.GC4938@sigill.intra.peff.net>

Hi Peff,

On Fri, 9 Nov 2018, Jeff King wrote:

> On Fri, Nov 09, 2018 at 06:21:41PM +0100, Johannes Schindelin wrote:
> 
> > 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.  :)

True. I simply prefer to initialize it and be done with it. ;-)

Ciao,
Dscho