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
