On Thu, Feb 27, 2025 at 07:36:42PM -0500, Taylor Blau wrote: > On Tue, Feb 25, 2025 at 01:34:21AM -0500, Jeff King wrote: > > This should make the logic a bit easier to follow. It does mean > > duplicating the buf cleanup for errors, but it's a single line. > > At least to my eyes, I actually prefer the state after 9/10 and would > probably be OK to see this patch get dropped. I wish I had a compelling > reason *why* I felt that way, but I think it may too subjective. > > I don't feel strongly about it either way, though. I also don't have a super strong feeling, though I fall on the other side of the line (which is why I bothered sending the patch). If we didn't do that, I think the alternative is probably a comment like: if (error1) error(describe error1); else if (error2) error(describe error2); else return buf; /* if we didn't return above, we saw some error */ free(buf); return NULL; I dunno. I'd probably stick with what I send. ;) -Peff
