From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 06 Jul 2021 19:28:40 GMT
Subject: Re: [PATCH v3 4/5] refs: add failure_errno to refs_read_raw_ref() signature
Message-ID: <xmqqk0m3jl7b.fsf@gitster.g>
In-Reply-To: <4aaa9d3bd6f01b83932b25a9ba18ec9c1106405a.1625518566.git.gitgitgadget@gmail.com>

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -	int result;
> -	int failure_errno;
> +	int ignore;
> +	if (failure_errno)
> +		*failure_errno = 0;
> +	else
> +		failure_errno = &ignore;

Hmph, I would have expected that a piece of code that gives fallback
location for ignored return parameter to initialize the location the
same way, i.e.

	int ignore

	if (!failure_errno)
		failure_errno = &ignore
	*failure_errno = 0;

That way, the code that follows that takes failure_errno does not
have to care if that return parameter location was supplied by the
caller or given by the fallback logic.

> +const char *refs_resolve_ref_unsafe_with_errno(struct ref_store *refs,
> +					       const char *refname,
> +					       int resolve_flags,
> +					       struct object_id *oid,
> +					       int *flags, int *failure_errno)
>  {
>  	static struct strbuf sb_refname = STRBUF_INIT;
>  	struct object_id unused_oid;
>  	int unused_flags;
> +	int unused_errno;
>  	int symref_count;
>  
>  	if (!oid)
> @@ -1707,6 +1708,9 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,
>  		flags = &unused_flags;
>  
>  	*flags = 0;
> +	if (!failure_errno)
> +		failure_errno = &unused_errno;
> +	*failure_errno = 0;

And you do use that pattern correctly here.  We probably would want
to be consistent.