Jeff King <peff@xxxxxxxx> 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@xxxxxxxx>
> ---
> 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;
> }
