Re: [PATCH v6 1/5] object-name: make get_oid quietly return an error

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

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> @@ -1081,13 +1081,17 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
>  				 * still fill in the oid with the "old" value,
>  				 * which we can use.
>  				 */

So far in this if/elseif cascade, we covered the case where we found
a reflog entry we are looking for before running out.  So ...

> -			} else {
> +			} else if (!(flags & GET_OID_GENTLY)) {
>  				if (flags & GET_OID_QUIETLY) {
>  					exit(128);
>  				}
>  				die(_("log for '%.*s' only has %d entries"),
>  				    len, str, co_cnt);
>  			}

... existing code chose between a silent exit or die based on
GET_OID_QUIETLY bit in the flags word.  In the updated code, this
block is entered only when the caller did not ask for
GET_OID_GENTLY.  But the point is that if we do not say GENTLY,
we no longer do this "choose between exit or die, either way we are
dead at this point".

OK.

> +			if (flags & GET_OID_GENTLY) {
> +				free(real_ref);
> +				return -1;
> +			}

I am confused.

Imagine that one of the if/elseif cascade handled the situation.
e.g. "The caller asked Nth, we found exactly N entries, so instead
of usual new side of the N-1th, we can give the old side of the Nth"
case is ready to return a success.  Why should the caller in such a
case instead get a failure only because the caller said "do not die
on me; I will handle failures myself"?

Shouldn't it be made the final "} else {" of the if/elseif cascade
instead?