From: Jeff King <peff@peff.net>
Date: Wed, 08 Jan 2014 10:23:48 GMT
Subject: Re: [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion
Message-ID: <20140108102348.GA30092@sigill.intra.peff.net>
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)) {