git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 01/18] rerere: fix an off-by-one non-bug

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 17, 2015, 22:24 UTC
Message-ID
<1437171880-21590-2-git-send-email-gitster@pobox.com>
In-Reply-To
<1437171880-21590-1-git-send-email-gitster@pobox.com>

When ac49f5ca (rerere "remaining", 2011-02-16) split out a new helper function check_one_conflict() out of find_conflict() function, so that the latter will use the returned value from the new helper to update the loop control variable that is an index into active_cache[], the new variable incremented the index by one too many when it found a path with only stage #1 entry at the very end of active_cache[].

This "strange" return value does not have any effect on the loop control of two callers of this function, as they all notice that active_nr+2 is larger than active_nr just like active_nr+1 is, but nevertheless it puzzles the readers when they are trying to figure out what the function is trying to do.

In fact, there is no need to do an early return. The code that follows after skipping the stage #1 entry is fully prepared to handle a case where the entry is at the very end of active_cache[].

Help future readers from unnecessary confusion by dropping an early return. We skip the stage #1 entry, and if there are stage #2 and stage #3 entries for the same path, we diagnose the path as THREE_STAGED (otherwise we say PUNTED), and then we skip all entries for the same path.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/rerere.c b/rerere.c
index 31644de..e307711 100644
--- a/rerere.c
+++ b/rerere.c
@@ -369,10 +369,8 @@ static int check_one_conflict(int i, int *type)
 	}
 
 	*type = PUNTED;
-	if (ce_stage(e) == 1) {
-		if (active_nr <= ++i)
-			return i + 1;
-	}
+	if (ce_stage(e) == 1)
+		i++;
 
 	/* Only handle regular files with both stages #2 and #3 */
 	if (i + 1 < active_nr) {
-- 
2.5.0-rc2-340-g0cccc16
Previous: Junio C HamanoNext: Junio C Hamano
Message 16 of 39 in “"rerere" minor clean-up”
  1. 00/13 "rerere" minor clean-upJunio C Hamano, Jul 1, 2015
  2. 01/13 rerere: fix an off-by-one non-bugJunio C Hamano, Jul 1, 2015
  3. 02/13 rerere: plug conflict ID leaksJunio C Hamano, Jul 1, 2015
  4. 03/13 rerere: lift PATH_MAX limitationJunio C Hamano, Jul 1, 2015
  5. 04/13 rerere: write out each record of MERGE_RR in one goJunio C Hamano, Jul 1, 2015
  6. 05/13 rerere: report autoupdated paths only after actually updating themJunio C Hamano, Jul 1, 2015
  7. 06/13 rerere: drop want_sp parameter from is_cmarker()Junio C Hamano, Jul 1, 2015
  8. 07/13 rerere: stop looping unnecessarilyJunio C Hamano, Jul 1, 2015
  9. 08/13 rerere: explain the rerere I/O abstractionJunio C Hamano, Jul 1, 2015
  10. 09/13 rerere: explain MERGE_RR management helpersJunio C Hamano, Jul 1, 2015
  11. 10/13 rerere: explain the primary codepathJunio C Hamano, Jul 1, 2015
  12. 11/13 rerere: explain "rerere forget" codepathJunio C Hamano, Jul 1, 2015
  13. 12/13 rerere: explain the remainderJunio C Hamano, Jul 1, 2015
  14. 13/13 rerere: refactor "replay" part of do_plain_rerere()Junio C Hamano, Jul 1, 2015
  15. 00/18 "rerere" preparatory clean-upJunio C Hamano, Jul 17, 2015
  16. 01/18 rerere: fix an off-by-one non-bugJunio C Hamano, Jul 17, 2015
  17. Junio C HamanoJul 24, 2015
  18. 02/18 rerere: plug conflict ID leaksJunio C Hamano, Jul 17, 2015
  19. 03/18 rerere: lift PATH_MAX limitationJunio C Hamano, Jul 17, 2015
  20. 04/18 rerere: write out each record of MERGE_RR in one goJunio C Hamano, Jul 17, 2015
  21. 05/18 rerere: report autoupdated paths only after actually updating themJunio C Hamano, Jul 17, 2015
  22. 06/18 rerere: drop want_sp parameter from is_cmarker()Junio C Hamano, Jul 17, 2015
  23. Philip OakleyJul 18, 2015
  24. Eric SunshineJul 18, 2015
  25. 07/18 rerere: stop looping unnecessarilyJunio C Hamano, Jul 17, 2015
  26. Junio C HamanoJul 24, 2015
  27. 08/18 rerere: explain the rerere I/O abstractionJunio C Hamano, Jul 17, 2015
  28. Junio C HamanoJul 24, 2015
  29. 09/18 rerere: explain MERGE_RR management helpersJunio C Hamano, Jul 17, 2015
  30. 10/18 rerere: explain the primary codepathJunio C Hamano, Jul 17, 2015
  31. 11/18 rerere: explain "rerere forget" codepathJunio C Hamano, Jul 17, 2015
  32. 12/18 rerere: explain the remainderJunio C Hamano, Jul 17, 2015
  33. 13/18 rerere: refactor "replay" part of do_plain_rerere()Junio C Hamano, Jul 17, 2015
  34. 14/18 rerere: further de-dent do_plain_rerere()Junio C Hamano, Jul 17, 2015
  35. 15/18 rerere: further clarify do_rerere_one_path()Junio C Hamano, Jul 17, 2015
  36. 16/18 rerere: call conflict-ids IDsJunio C Hamano, Jul 17, 2015
  37. 17/18 rerere: use "struct rerere_id" instead of "char *" for conflict IDJunio C Hamano, Jul 17, 2015
  38. Eric SunshineJul 18, 2015
  39. 18/18 rerere: un-nest merge() furtherJunio C Hamano, Jul 17, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.