From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 28 Oct 2022 21:49:09 GMT
Subject: Re: [PATCH v3 5/8] submodule: return target of submodule symref
Message-ID: <xmqqfsf7hb56.fsf@gitster.g>
In-Reply-To: <bd8ffd7cde2feb3853f837778d660c0f47db32ff.1666988096.git.gitgitgadget@gmail.com>

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  int resolve_gitlink_ref(const char *submodule, const char *refname,
> -			struct object_id *oid)
> +			struct object_id *oid, const char **target_out)
>  {
>  	struct ref_store *refs;
>  	int flags;
> +	const char *target;
>  
>  	refs = get_submodule_ref_store(submodule);
>  
>  	if (!refs)
>  		return -1;
> -
> -	if (!refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags) ||
> -	    is_null_oid(oid))
> +	target = refs_resolve_ref_unsafe(refs, refname, 0, oid, &flags);
> +	if (!target || is_null_oid(oid))
>  		return -1;
> +	if (target_out)
> +		*target_out = target;
>  	return 0;
>  }

Please remind me why we call this underlying helper _unsafe()?

Isn't it because we return a temporary buffer  (static strbuf),
whose contents is not permanent?

I am wondering if we should force the callers who care enough to
pass the optional **target_out to xstrdup() the result.