Re: [PATCH 3/3] Rename resolve_ref() to resolve_ref_unsafe()

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 10, 2011, 20:55 UTC
Message-ID: <20111210205519.GA24817@elie.hsd1.il.comcast.net>
In-Reply-To: <1323521631-24320-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

> resolve_ref() may return a pointer to a shared buffer and can be
> overwritten by the next resolve_ref() calls. Callers need to
> pay attention, not to keep the pointer when the next call happens.

[...]

Show 8 quoted lines

> --- a/branch.c
> +++ b/branch.c
> @@ -182,7 +182,7 @@ int validate_new_branchname(const char *name, struct strbuf *ref,
>  		const char *head;
>  		unsigned char sha1[20];
>  
> -		head = resolve_ref("HEAD", sha1, 0, NULL);
> +		head = resolve_ref_unsafe("HEAD", sha1, 0, NULL);

I wonder if it would make sense to have a separate function that maintains a shared buffer describing what HEAD resolves to, lazily loaded. Would invalidating the cache when appropriate be too fussy and subtle?

[...]

Show 7 quoted lines

> +++ b/transport.c
> @@ -163,7 +163,7 @@ static void set_upstreams(struct transport *transport, struct ref *refs,
>  		/* Follow symbolic refs (mainly for HEAD). */
>  		localname = ref->peer_ref->name;
>  		remotename = ref->name;
> -		tmp = resolve_ref(localname, sha, 1, &flag);
> +		tmp = resolve_ref_unsafe(localname, sha, 1, &flag);

Anyway, this patch looks sane. The reminder seems useful and doesn't feel over-the-top.