All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH v2 13/33] peel_ref(): fix return value for non-peelable, not-current reference
Date: Mon, 22 Apr 2013 21:52:21 +0200	[thread overview]
Message-ID: <1366660361-21831-14-git-send-email-mhagger@alum.mit.edu> (raw)
In-Reply-To: <1366660361-21831-1-git-send-email-mhagger@alum.mit.edu>

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 only call peel_ref() from within a for_each_ref-style
iteration and only for the current ref; therefore, the buggy code path
was never reached.

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 d26e847..2f73189 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. **/
-- 
1.8.2.1

  parent reply	other threads:[~2013-04-22 19:54 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-22 19:52 [PATCH v2 00/33] Various cleanups around reference packing and peeling Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 01/33] refs: document flags constants REF_* Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 02/33] refs: document the fields of struct ref_value Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 03/33] refs: document do_for_each_ref() and do_one_ref() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 04/33] refs: document how current_ref is used Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 05/33] refs: define constant PEELED_LINE_LENGTH Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 06/33] do_for_each_ref_in_dirs(): remove dead code Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 07/33] get_packed_ref(): return a ref_entry Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 08/33] peel_ref(): use function get_packed_ref() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 09/33] repack_without_ref(): " Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 10/33] refs: extract a function ref_resolves_to_object() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 11/33] refs: extract function peel_object() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 12/33] peel_object(): give more specific information in return value Michael Haggerty
2013-04-22 19:52 ` Michael Haggerty [this message]
2013-04-22 19:52 ` [PATCH v2 14/33] refs: extract a function peel_entry() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 15/33] refs: change the internal reference-iteration API Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 16/33] t3210: test for spurious error messages for dangling packed refs Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 17/33] repack_without_ref(): silence errors " Michael Haggerty
2013-04-22 22:21   ` Junio C Hamano
2013-04-22 19:52 ` [PATCH v2 18/33] search_ref_dir(): return an index rather than a pointer Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 19/33] refs: change how packed refs are deleted Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 20/33] t3211: demonstrate loss of peeled refs if a packed ref is deleted Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 21/33] repack_without_ref(): write peeled refs in the rewritten file Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 22/33] refs: extract a function write_packed_entry() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 23/33] pack-refs: rename handle_one_ref() to pack_one_ref() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 24/33] pack-refs: merge code from pack-refs.{c,h} into refs.{c,h} Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 25/33] pack_one_ref(): rename "path" parameter to "refname" Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 26/33] refs: use same lock_file object for both ref-packing functions Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 27/33] pack_refs(): change to use do_for_each_entry() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 28/33] refs: inline function do_not_prune() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 29/33] pack_one_ref(): use function peel_entry() Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 30/33] pack_one_ref(): use write_packed_entry() to do the writing Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 31/33] pack_one_ref(): do some cheap tests before a more expensive one Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 32/33] refs: change do_for_each_*() functions to take ref_cache arguments Michael Haggerty
2013-04-22 19:52 ` [PATCH v2 33/33] refs: handle the main ref_cache specially Michael Haggerty
2013-04-22 22:23 ` [PATCH v2 00/33] Various cleanups around reference packing and peeling Junio C Hamano
2013-04-23  9:15   ` [PATCH] fixup! t3210: test for spurious error messages for dangling packed refs Michael Haggerty
2013-04-23 17:50     ` Junio C Hamano
2013-04-24 15:25       ` Michael Haggerty
2013-04-27  5:43         ` Michael Haggerty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1366660361-21831-14-git-send-email-mhagger@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.