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: Re: [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion
Date: Wed, 8 Jan 2014 05:23:48 -0500	[thread overview]
Message-ID: <20140108102348.GA30092@sigill.intra.peff.net> (raw)
In-Reply-To: <20140108034733.GA17198@sigill.intra.peff.net>

On Tue, Jan 07, 2014 at 10:47:33PM -0500, Jeff King wrote:

> On Tue, Jan 07, 2014 at 06:58:50PM -0500, Jeff King wrote:
> 
> > +			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,
> 
> Obviously this is totally wrong and inverts the point of the flag. And
> causes something like half of the test suite to fail.

And while we're on the subject of my mistakes...

The patch needs the fixup below to ensure that retval is always set,
even when we do not recurse.

I'll hold off on sending a full re-roll of the patch, in the extremely
unlikely event that there are other small errors to be fixed. :)

diff --git a/refs.c b/refs.c
index aafbae9..99c72d0 100644
--- a/refs.c
+++ b/refs.c
@@ -679,7 +679,8 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
 				retval = do_for_each_entry_in_dir(subdir, 0,
 								  fn, cb_data,
 								  flags);
-			}
+			} else
+				retval = 0;
 		} else {
 			retval = fn(entry, cb_data);
 		}
@@ -732,7 +733,8 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
 					retval = do_for_each_entry_in_dirs(
 							subdir1, subdir2,
 							fn, cb_data, flags);
-				}
+				} else
+					retval = 0;
 				i1++;
 				i2++;
 			} else if (!(e1->flag & REF_DIR) && !(e2->flag & REF_DIR)) {

  reply	other threads:[~2014-01-08 10:23 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                   ` [PATCH v2 3/5] refs: teach for_each_ref a flag to avoid recursion Jeff King
2014-01-08  3:47                     ` [PATCH v3 " Jeff King
2014-01-08 10:23                       ` Jeff King [this message]
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=20140108102348.GA30092@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).