Re: [PATCH v3 5/8] submodule: return target of submodule symref

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 28, 2022, 21:49 UTC
Message-ID: <xmqqfsf7hb56.fsf@gitster.g>
In-Reply-To: <bd8ffd7cde2feb3853f837778d660c0f47db32ff.1666988096.git.gitgitgadget@gmail.com>

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

Show 22 quoted lines

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