Re: [PATCH 13/33] peel_ref(): fix return value for non-peelable, not-current reference
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 15, 2013, 17:38 UTC
- Message-ID
- <7va9ozwv70.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1365944088-10588-14-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 7 quoted lines
> The old version was inconsistent: when a reference was > REF_KNOWS_PEELED but with a null peeled value, it returned non-zero > for the current reference but zero for other references. Change the > behavior for non-current references to match that of current_ref, > which is what callers expect. Document the behavior. > > Current callers did not trigger the previously-buggy behavior.
Is that because we were lucky by codeflow, or is it just that we didn't have a testcase to trigger the behaviour?
Show 49 quoted lines
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 5 ++++-
> refs.h | 8 ++++++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index 84c2497..a0d8e32 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -120,7 +120,8 @@ struct ref_value {
> /*
> * If REF_KNOWS_PEELED, then this field holds the peeled value
> * of this reference, or null if the reference is known not to
> - * be peelable.
> + * be peelable. See the documentation for peel_ref() for an
> + * exact definition of "peelable".
> */
> unsigned char peeled[20];
> };
> @@ -1339,6 +1340,8 @@ int peel_ref(const char *refname, unsigned char *sha1)
> struct ref_entry *r = get_packed_ref(refname);
>
> if (r && (r->flag & REF_KNOWS_PEELED)) {
> + if (is_null_sha1(r->u.value.peeled))
> + return -1;
> hashcpy(sha1, r->u.value.peeled);
> return 0;
> }
> diff --git a/refs.h b/refs.h
> index 17bc1c1..ac0ff92 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -74,6 +74,14 @@ extern void add_packed_ref(const char *refname, const unsigned char *sha1);
>
> extern int ref_exists(const char *);
>
> +/*
> + * If refname is a non-symbolic reference that refers to a tag object,
> + * and the tag can be (recursively) dereferenced to a non-tag object,
> + * store the SHA1 of the referred-to object to sha1 and return 0. If
> + * any of these conditions are not met, return a non-zero value.
> + * Symbolic references are considered unpeelable, even if they
> + * ultimately resolve to a peelable tag.
> + */
> extern int peel_ref(const char *refname, unsigned char *sha1);
>
> /** Locks a "refs/" ref returning the lock on success and NULL on failure. **/