Re: [PATCH 09/10] unpack_loose_rest(): simplify error handling
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 26, 2025, 13:46 UTC
- Message-ID
- <xmqqldtsg6oz.fsf@gitster.g>
- In-Reply-To
- <20250225063351.GI1293961@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 34 quoted lines
> Inflating a loose object is considered successful only if we got > Z_STREAM_END and there were no more bytes. We check both of those > conditions and return success, but then have to check them a second time > to decide which error message to produce. > > I.e., we do something like this: > > if (!error_1 && !error_2) > ...return success... > > if (error_1) > ...handle error1... > else if (error_2) > ...handle error2... > ...common error handling... > > This repetition was the source of a small bug fixed in an earlier commit > (our Z_STREAM_END check was not the same in the two conditionals). > > Instead we can chain them all into a single if/else cascade, which > avoids repeating ourselves: > > if (error_1) > ...handle error1... > else if (error_2) > ...handle error2.... > else > ...return success... > ...common error handling... > > Signed-off-by: Jeff King <peff@peff.net> > --- > object-file.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-)
Of course the resulting code is so much cleaner and more obvious. Thanks for cleaning it up.
Show 23 quoted lines
> diff --git a/object-file.c b/object-file.c
> index 8cf87caef5..b7928fb74e 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1436,15 +1436,15 @@ static void *unpack_loose_rest(git_zstream *stream,
> obj_read_lock();
> }
> }
> - if (status == Z_STREAM_END && !stream->avail_in) {
> - return buf;
> - }
>
> if (status != Z_STREAM_END)
> error(_("corrupt loose object '%s'"), oid_to_hex(oid));
> else if (stream->avail_in)
> error(_("garbage at end of loose object '%s'"),
> oid_to_hex(oid));
> + else
> + return buf;
> +
> free(buf);
> return NULL;
> }