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. :) -Peff
