Re: [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion

From: Jeff King <peff@peff.net>
Date: Jan 8, 2014, 10:23 UTC
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:

Show 9 quoted lines

> 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)) {