Re: [PATCH 10/10] unpack_loose_rest(): rewrite return handling for clarity

From: Jeff King <peff@peff.net>
Date: Mar 4, 2025, 07:10 UTC
Message-ID: <20250304071011.GB1283943@coredump.intra.peff.net>
In-Reply-To: <Z8EFGnUmyAAJ6DUY@nand.local>

On Thu, Feb 27, 2025 at 07:36:42PM -0500, Taylor Blau wrote:

Show 9 quoted lines

> 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