git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Brodie Rao" <brodie@sf.io>,
	git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Michael Haggerty" <mhagger@alum.mit.edu>
Subject: [PATCH v2 3/5] refs: teach for_each_ref a flag to avoid recursion
Date: Tue, 7 Jan 2014 18:58:50 -0500	[thread overview]
Message-ID: <20140107235850.GC10657@sigill.intra.peff.net> (raw)
In-Reply-To: <20140107235631.GA10503@sigill.intra.peff.net>

The normal for_each_ref traversal descends into
subdirectories, returning each ref it finds. However, in
some cases we may want to just iterate over the top-level of
a certain part of the tree.

The introduction of the "flags" option is a little
mysterious. We already have a "flags" option that gets stuck
in a callback struct and ends up interpreted in do_one_ref.
But the traversal itself does not currently have any flags,
and it needs to know about this new flag.

We _could_ introduce this as a completely separate flag
parameter. But instead, we simply put both flag types into a
single namespace, and make it available at both sites. This
is simple, and given that we do not have a proliferation of
flags (we have had exactly one until now), it is probably
sufficient.

Signed-off-by: Jeff King <peff@peff.net>
---
I think the flags thing is OK as explained above, but Michael may have a
different suggestion for refactoring.

 refs.c | 61 ++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/refs.c b/refs.c
index 3926136..ca854d6 100644
--- a/refs.c
+++ b/refs.c
@@ -589,6 +589,8 @@ static void sort_ref_dir(struct ref_dir *dir)
 
 /* Include broken references in a do_for_each_ref*() iteration: */
 #define DO_FOR_EACH_INCLUDE_BROKEN 0x01
+/* Do not recurse into subdirs, just iterate at a single level. */
+#define DO_FOR_EACH_NO_RECURSE     0x02
 
 /*
  * Return true iff the reference described by entry can be resolved to
@@ -661,7 +663,8 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)
  * called for all references, including broken ones.
  */
 static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
-				    each_ref_entry_fn fn, void *cb_data)
+				    each_ref_entry_fn fn, void *cb_data,
+				    int flags)
 {
 	int i;
 	assert(dir->sorted == dir->nr);
@@ -669,9 +672,13 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
 		struct ref_entry *entry = dir->entries[i];
 		int retval;
 		if (entry->flag & REF_DIR) {
-			struct ref_dir *subdir = get_ref_dir(entry);
-			sort_ref_dir(subdir);
-			retval = do_for_each_entry_in_dir(subdir, 0, fn, cb_data);
+			if (flags & DO_FOR_EACH_NO_RECURSE) {
+				struct ref_dir *subdir = get_ref_dir(entry);
+				sort_ref_dir(subdir);
+				retval = do_for_each_entry_in_dir(subdir, 0,
+								  fn, cb_data,
+								  flags);
+			}
 		} else {
 			retval = fn(entry, cb_data);
 		}
@@ -691,7 +698,8 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
  */
 static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
 				     struct ref_dir *dir2,
-				     each_ref_entry_fn fn, void *cb_data)
+				     each_ref_entry_fn fn, void *cb_data,
+				     int flags)
 {
 	int retval;
 	int i1 = 0, i2 = 0;
@@ -702,10 +710,12 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
 		struct ref_entry *e1, *e2;
 		int cmp;
 		if (i1 == dir1->nr) {
-			return do_for_each_entry_in_dir(dir2, i2, fn, cb_data);
+			return do_for_each_entry_in_dir(dir2, i2, fn, cb_data,
+							flags);
 		}
 		if (i2 == dir2->nr) {
-			return do_for_each_entry_in_dir(dir1, i1, fn, cb_data);
+			return do_for_each_entry_in_dir(dir1, i1, fn, cb_data,
+							flags);
 		}
 		e1 = dir1->entries[i1];
 		e2 = dir2->entries[i2];
@@ -713,12 +723,15 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
 		if (cmp == 0) {
 			if ((e1->flag & REF_DIR) && (e2->flag & REF_DIR)) {
 				/* Both are directories; descend them in parallel. */
-				struct ref_dir *subdir1 = get_ref_dir(e1);
-				struct ref_dir *subdir2 = get_ref_dir(e2);
-				sort_ref_dir(subdir1);
-				sort_ref_dir(subdir2);
-				retval = do_for_each_entry_in_dirs(
-						subdir1, subdir2, fn, cb_data);
+				if (!(flags & DO_FOR_EACH_NO_RECURSE)) {
+					struct ref_dir *subdir1 = get_ref_dir(e1);
+					struct ref_dir *subdir2 = get_ref_dir(e2);
+					sort_ref_dir(subdir1);
+					sort_ref_dir(subdir2);
+					retval = do_for_each_entry_in_dirs(
+							subdir1, subdir2,
+							fn, cb_data, flags);
+				}
 				i1++;
 				i2++;
 			} else if (!(e1->flag & REF_DIR) && !(e2->flag & REF_DIR)) {
@@ -743,7 +756,7 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
 				struct ref_dir *subdir = get_ref_dir(e);
 				sort_ref_dir(subdir);
 				retval = do_for_each_entry_in_dir(
-						subdir, 0, fn, cb_data);
+						subdir, 0, fn, cb_data, flags);
 			} else {
 				retval = fn(e, cb_data);
 			}
@@ -817,7 +830,7 @@ static int is_refname_available(const char *refname, const char *oldrefname,
 	data.conflicting_refname = NULL;
 
 	sort_ref_dir(dir);
-	if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, &data)) {
+	if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, &data, 0)) {
 		error("'%s' exists; cannot create '%s'",
 		      data.conflicting_refname, refname);
 		return 0;
@@ -1651,7 +1664,8 @@ void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
  * 0.
  */
 static int do_for_each_entry(struct ref_cache *refs, const char *base,
-			     each_ref_entry_fn fn, void *cb_data)
+			     each_ref_entry_fn fn, void *cb_data,
+			     int flags)
 {
 	struct packed_ref_cache *packed_ref_cache;
 	struct ref_dir *loose_dir;
@@ -1684,15 +1698,15 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,
 		sort_ref_dir(packed_dir);
 		sort_ref_dir(loose_dir);
 		retval = do_for_each_entry_in_dirs(
-				packed_dir, loose_dir, fn, cb_data);
+				packed_dir, loose_dir, fn, cb_data, flags);
 	} else if (packed_dir) {
 		sort_ref_dir(packed_dir);
 		retval = do_for_each_entry_in_dir(
-				packed_dir, 0, fn, cb_data);
+				packed_dir, 0, fn, cb_data, flags);
 	} else if (loose_dir) {
 		sort_ref_dir(loose_dir);
 		retval = do_for_each_entry_in_dir(
-				loose_dir, 0, fn, cb_data);
+				loose_dir, 0, fn, cb_data, flags);
 	}
 
 	release_packed_ref_cache(packed_ref_cache);
@@ -1718,7 +1732,7 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
 	data.fn = fn;
 	data.cb_data = cb_data;
 
-	return do_for_each_entry(refs, base, do_one_ref, &data);
+	return do_for_each_entry(refs, base, do_one_ref, &data, flags);
 }
 
 static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
@@ -2200,7 +2214,7 @@ int commit_packed_refs(void)
 
 	do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
 				 0, write_packed_entry_fn,
-				 &packed_ref_cache->lock->fd);
+				 &packed_ref_cache->lock->fd, 0);
 	if (commit_lock_file(packed_ref_cache->lock))
 		error = -1;
 	packed_ref_cache->lock = NULL;
@@ -2345,7 +2359,7 @@ int pack_refs(unsigned int flags)
 	cbdata.packed_refs = get_packed_refs(&ref_cache);
 
 	do_for_each_entry_in_dir(get_loose_refs(&ref_cache), 0,
-				 pack_if_possible_fn, &cbdata);
+				 pack_if_possible_fn, &cbdata, 0);
 
 	if (commit_packed_refs())
 		die_errno("unable to overwrite old ref-pack file");
@@ -2447,7 +2461,8 @@ static int repack_without_refs(const char **refnames, int n)
 	}
 
 	/* Remove any other accumulated cruft */
-	do_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn, &refs_to_delete);
+	do_for_each_entry_in_dir(packed, 0, curate_packed_ref_fn,
+				 &refs_to_delete, 0);
 	for_each_string_list_item(ref_to_delete, &refs_to_delete) {
 		if (remove_entry(packed, ref_to_delete->string) == -1)
 			die("internal error");
-- 
1.8.5.2.500.g8060133

  parent reply	other threads:[~2014-01-07 23:58 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-07  3:32 [PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false Brodie Rao
2014-01-07  3:35 ` Brodie Rao
2014-01-07 17:13   ` Jeff King
2014-01-07 17:51     ` Junio C Hamano
2014-01-07 17:52       ` Jeff King
2014-01-07 19:38         ` Junio C Hamano
2014-01-07 19:58           ` Jeff King
2014-01-07 20:31             ` Junio C Hamano
2014-01-07 22:08               ` Jeff King
2014-01-07 22:10                 ` [PATCH 1/4] cat-file: refactor error handling of batch_objects Jeff King
2014-01-07 22:10                 ` [PATCH 2/4] cat-file: fix a minor memory leak in batch_objects Jeff King
2014-01-07 22:10                 ` [PATCH 3/4] cat-file: restore ambiguity warning flag " Jeff King
2014-01-07 22:11                 ` [PATCH 4/4] revision: turn off object/refname ambiguity check for --stdin Jeff King
2014-01-07 23:56                 ` [PATCH v2] speeding up 40-hex ambiguity check Jeff King
2014-01-07 23:57                   ` [PATCH v2 1/5] cat-file: refactor error handling of batch_objects Jeff King
2014-01-07 23:57                   ` [PATCH v2 2/5] cat-file: fix a minor memory leak in batch_objects Jeff King
2014-01-07 23:58                   ` Jeff King [this message]
2014-01-08  3:47                     ` [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion Jeff King
2014-01-08 10:23                       ` Jeff King
2014-01-08 11:29                       ` Michael Haggerty
2014-01-09 21:49                         ` Jeff King
2014-01-10  8:59                           ` Michael Haggerty
2014-01-10  9:15                             ` Jeff King
2014-01-09 17:51                       ` Junio C Hamano
2014-01-09 21:55                         ` Jeff King
2014-01-07 23:59                   ` [PATCH v2 4/5] get_sha1: speed up ambiguous 40-hex test Jeff King
2014-01-08 16:09                     ` Michael Haggerty
2014-01-09 18:25                       ` Junio C Hamano
2014-01-10  9:41                       ` Jeff King
2014-01-14  9:50                         ` Jeff King
2014-01-14 11:34                           ` Michael Haggerty
2014-01-08  0:00                   ` [PATCH v2 5/5] get_sha1: drop object/refname ambiguity flag Jeff King
2014-01-08 16:34                     ` Michael Haggerty
2014-01-07  6:45 ` [PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false Duy Nguyen
2014-01-07 17:24 ` Junio C Hamano
2014-01-07 19:23   ` Brodie Rao

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=20140107235850.GC10657@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=brodie@sf.io \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    --cc=pclouds@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).