On Thu, Dec 04, 2014 at 08:28:54PM -0500, Jeff King wrote:
> The minimal fix here would be to add this special case to
> the conditional that checks whether we found a newline.
> But we can make the flow a little clearer by rearranging a
> bit: we first handle lines that we are going to show, and
> then at the end of each loop, stuff away any leftovers if
> necessary. That lets us fold this special-case in with the
> more common "we ended in the middle of a line" case.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> I really needed this rearranging to figure out what the fix
> _should_ be. Now that I did that, I was able to write the above
> paragraph explaining what the minimal fix would be. And I can do that if
> we prefer. But I think the fact that I had to go through the untangling
> step first is an indication that maybe the end result is better. Of
> course that's all subjective. :)
I _think_ the patch below is also applicable to the code before my
boundary-fixing patch. But the rearranging also made me more confident
in it.
-- >8 --
Subject: for_each_reflog_ent_reverse: turn leftover check into assertion
Our loop should always process all lines, even if we hit the
beginning of the file. We have a conditional after the loop
ends to double-check that there is nothing left and to
process it. But this should never happen, and is a sign of a
logic bug in the loop. Let's turn it into a BUG assertion.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
Of course I cannot say something like "this can never happen; the old
code was wrong to handle this case" without a nagging feeling that I am
missing something, so extra careful eyes are appreciated (and are why I
would rather have an assert here than removing the code and silently
dropping lines if I am wrong).
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index ccb8834..1f77fa6 100644
--- a/refs.c
+++ b/refs.c
@@ -3451,7 +3451,7 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void
}
if (!ret && sb.len)
- ret = show_one_reflog_ent(&sb, fn, cb_data);
+ die("BUG: reverse reflog parser had leftover data");
fclose(logfp);
strbuf_release(&sb);
--
2.2.0.390.gf60752d
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html