{"thread":{"id":"60153","subject":"[PATCH 0/4] code clean-up for rerere","startedAt":"2023-08-24T20:56:03Z","lastAt":"2023-08-24T20:56:04Z","messageCount":5,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"481014","messageId":"20230824205456.1231371-1-gitster@pobox.com","threadId":"60153","inReplyTo":null,"subject":"[PATCH 0/4] code clean-up for rerere","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-24T20:54:52Z","receivedAt":"2023-08-24T20:56:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"These are to clean up some odd code and in-code comment that invited\n\"Huh?\" while I was reviewing the code paths involved in \"git rerere\"\nrecently. None of them is intended to change any behaviour.\n\nJunio C Hamano (4):\n rerere: simplify check_one_conflict() helper function\n rerere: fix comment on handle_file() helper\n rerere: try_merge() should use LL_MERGE_ERROR when it means an error\n rerere: modernize use of empty strbuf\n\n rerere.c | 46 +++++++++++++++++++---------------------------\n 1 file changed, 19 insertions(+), 27 deletions(-)\n\n-- \n2.42.0-29-gcd9da15a85\n\n"},{"id":"481015","messageId":"20230824205456.1231371-4-gitster@pobox.com","threadId":"60153","inReplyTo":"20230824205456.1231371-1-gitster@pobox.com","subject":"[PATCH 3/4] rerere: try_merge() should use LL_MERGE_ERROR when it means an error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-24T20:54:55Z","receivedAt":"2023-08-24T20:56:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When the preimage or the postimage files cannot be read, the\ntry_merge() helper function returns LL_MERGE_CONFLICT. To all of\nits callers, this does not make them do wrong things per-se, as they\nare only checking if the result is 0 and LL_MERGE_CONFLICT is not 0.\n\nBut it is an error if we fail to read the input we expect to be able\nto read; return LL_MERGE_ERROR instead. This does not change any\nbehaviour---it just makes the code use the \"correct\" constant to\nsignal an error.\n\nSigned-off-by: Junio C Hamano \n---\n rerere.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 4ce1270a94..6bc3c54d3b 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -617,7 +617,7 @@ static int try_merge(struct index_state *istate,\n \n \tif (read_mmfile(&base, rerere_path(id, \"preimage\")) ||\n \t read_mmfile(&other, rerere_path(id, \"postimage\"))) {\n-\t\tret = LL_MERGE_CONFLICT;\n+\t\tret = LL_MERGE_ERROR;\n \t} else {\n \t\t/*\n \t\t * A three-way merge. Note that this honors user-customizable\n-- \n2.42.0-29-gcd9da15a85\n\n"},{"id":"481016","messageId":"20230824205456.1231371-2-gitster@pobox.com","threadId":"60153","inReplyTo":"20230824205456.1231371-1-gitster@pobox.com","subject":"[PATCH 1/4] rerere: simplify check_one_conflict() helper function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-24T20:54:53Z","receivedAt":"2023-08-24T20:56:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The helper function is responsible for inspecting the index and\ndeciding if the path is merged, is conflicted in a way that we would\nwant to handle, or is conflicted in a way that we cannot handle.\n\nCurrently, only conflicts with both stage #2 and stage #3 are\nhandled, but eventually we would want to be able to deal with\ndelete-modify conflicts (i.e. only one of stages #2 and #3 exist).\nStreamline the implementation of the function to make it easier to\nextend.\n\nSigned-off-by: Junio C Hamano \n---\n rerere.c | 34 +++++++++++++++-------------------\n 1 file changed, 15 insertions(+), 19 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex b525dd9230..73cdc8392f 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -499,9 +499,11 @@ static int handle_file(struct index_state *istate,\n }\n \n /*\n- * Look at a cache entry at \"i\" and see if it is not conflicting,\n- * conflicting and we are willing to handle, or conflicting and\n- * we are unable to handle, and return the determination in *type.\n+ * Look at a cache entry at \"i\" and see if it is not conflicting\n+ * (RESOLVED), conflicting and we are willing to handle (THREE_STAGED),\n+ * or conflicting and we are unable to handle (PUNTED), and return the\n+ * determination in *type.\n+ *\n * Return the cache index to be looked at next, by skipping the\n * stages we have already looked at in this invocation of this\n * function.\n@@ -509,6 +511,7 @@ static int handle_file(struct index_state *istate,\n static int check_one_conflict(struct index_state *istate, int i, int *type)\n {\n \tconst struct cache_entry *e = istate->cache[i];\n+\tunsigned int seen_stages = 0;\n \n \tif (!ce_stage(e)) {\n \t\t*type = RESOLVED;\n@@ -516,24 +519,17 @@ static int check_one_conflict(struct index_state *istate, int i, int *type)\n \t}\n \n \t*type = PUNTED;\n-\twhile (i < istate->cache_nr && ce_stage(istate->cache[i]) == 1)\n-\t\ti++;\n-\n-\t/* Only handle regular files with both stages #2 and #3 */\n-\tif (i + 1 < istate->cache_nr) {\n-\t\tconst struct cache_entry *e2 = istate->cache[i];\n-\t\tconst struct cache_entry *e3 = istate->cache[i + 1];\n-\t\tif (ce_stage(e2) == 2 &&\n-\t\t ce_stage(e3) == 3 &&\n-\t\t ce_same_name(e, e3) &&\n-\t\t S_ISREG(e2->ce_mode) &&\n-\t\t S_ISREG(e3->ce_mode))\n-\t\t\t*type = THREE_STAGED;\n+\tfor (; i < istate->cache_nr; i++) {\n+\t\tconst struct cache_entry *n = istate->cache[i];\n+\t\tif (!ce_same_name(n, e))\n+\t\t\tbreak;\n+\t\tif (S_ISREG(n->ce_mode))\n+\t\t\tseen_stages |= 1u << (ce_stage(n) - 1);\n \t}\n \n-\t/* Skip the entries with the same name */\n-\twhile (i < istate->cache_nr && ce_same_name(e, istate->cache[i]))\n-\t\ti++;\n+\tif ((seen_stages & 6) == 6)\n+\t\t*type = THREE_STAGED; /* has both stages #2 and #3 */\n+\n \treturn i;\n }\n \n-- \n2.42.0-29-gcd9da15a85\n\n"},{"id":"481017","messageId":"20230824205456.1231371-5-gitster@pobox.com","threadId":"60153","inReplyTo":"20230824205456.1231371-1-gitster@pobox.com","subject":"[PATCH 4/4] rerere: modernize use of empty strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-24T20:54:56Z","receivedAt":"2023-08-24T20:56:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Back when the code in the handle_conflict() helper function that\nhashes the contents stored in the strbuf \"one\" and \"two\", including\nits terminating NUL, was written, a freshly initialized strbuf had\nNULL in its .buf member, so it was an error to say \n\n update(one.buf, one.len + 1)\n\nwhich was corrected by b4833a2c (rerere: Fix use of an empty\nstrbuf.buf, 2007-09-26).\n\nBut soon after that, b315c5c0 (strbuf change: be sure ->buf is never\never NULL., 2007-09-27) introduced strbuf_slopbuf mechanism that\nensures that .buf member of a strbuf is *never* NULL. A freshly\ninitialized and empty strbuf uses a static piece of memory that has\nNUL in it, with its .len member set to 0, so we can always safely\nuse from offset 0 of .buf[] array for (one.len + 1) bytes.\n\nSimplify the code by essentially reverting the b4833a2c, whose fix\nis no longer necessary in the modern world order.\n\nSigned-off-by: Junio C Hamano \n---\n rerere.c | 8 ++------\n 1 file changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 6bc3c54d3b..69a61aac93 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -390,12 +390,8 @@ static int handle_conflict(struct strbuf *out, struct rerere_io *io,\n \t\t\tstrbuf_addbuf(out, &two);\n \t\t\trerere_strbuf_putconflict(out, '>', marker_size);\n \t\t\tif (ctx) {\n-\t\t\t\tthe_hash_algo->update_fn(ctx, one.buf ?\n-\t\t\t\t\t\t\t one.buf : \"\",\n-\t\t\t\t\t\t\t one.len + 1);\n-\t\t\t\tthe_hash_algo->update_fn(ctx, two.buf ?\n-\t\t\t\t\t\t\t two.buf : \"\",\n-\t\t\t\t\t\t\t two.len + 1);\n+\t\t\t\tthe_hash_algo->update_fn(ctx, one.buf, one.len + 1);\n+\t\t\t\tthe_hash_algo->update_fn(ctx, two.buf, two.len + 1);\n \t\t\t}\n \t\t\tbreak;\n \t\t} else if (hunk == RR_SIDE_1)\n-- \n2.42.0-29-gcd9da15a85\n\n"},{"id":"481018","messageId":"20230824205456.1231371-3-gitster@pobox.com","threadId":"60153","inReplyTo":"20230824205456.1231371-1-gitster@pobox.com","subject":"[PATCH 2/4] rerere: fix comment on handle_file() helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-24T20:54:54Z","receivedAt":"2023-08-24T20:56:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The return value from handle_path() is returned to the caller of\nhandle_file() in the normal cases, and it is not the number of\nhunks. It is just a normal C Boolean, \"do we (!=0) or do we not (0)\nhave conflict?\" plus \"a negative return value signals an error\".\n\nAnd all the callers of handle_file() understands its return value as\nsuch. Update the comment to match the reality after 221444f5\n(rerere: only return whether a path has conflicts or not,\n2018-08-05), which apparently forgot to update this comment when it\nturned the returned value of this function from the number of\nconflict hunks to a boolean plus error.\n\nSigned-off-by: Junio C Hamano \n\n---\ncc: Thomas Gummerer \n---\n rerere.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 73cdc8392f..4ce1270a94 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -454,7 +454,7 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz\n \n /*\n * Scan the path for conflicts, do the \"handle_path()\" thing above, and\n- * return the number of conflict hunks found.\n+ * return true iff conflict hunks were found.\n */\n static int handle_file(struct index_state *istate,\n \t\t const char *path, unsigned char *hash, const char *output)\n-- \n2.42.0-29-gcd9da15a85\n\n"}]}