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

From: Junio C Hamano <gitster@pobox.com>
Date: May 22, 2025, 19:27 UTC
Message-ID: <xmqqa5745ttw.fsf@gitster.g>
In-Reply-To: <20250522185524.18398-3-sandals@crustytoothpaste.net>

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

Show 8 quoted lines

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