Re: [PATCH 09/10] unpack_loose_rest(): simplify error handling

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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.


> 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;
>  }