{"thread":{"id":"64883","subject":"[PATCH] path: refactor normalize_path_copy_len()","startedAt":"2026-01-29T15:15:19Z","lastAt":"2026-02-21T11:06:50Z","messageCount":6,"participants":["Pushkar Singh","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"534815","messageId":"20260129145434.29123-2-pushkarkumarsingh1970@gmail.com","threadId":"64883","inReplyTo":null,"subject":"[PATCH] path: refactor normalize_path_copy_len()","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-29T14:54:35Z","receivedAt":"2026-01-29T15:15:19Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Refactor normalize_path_copy_len() by extracting helpers for skipping\nslashes, handling dot components, and stripping the previous path\ncomponent, making the control flow easier to follow.\n\nThis is a mechanical refactor only; there are no functional changes.\nBehavior is unchanged, as verified by t0060-path-utils.sh.\n\nSigned-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n---\n path.c | 105 ++++++++++++++++++++++++++++++++++++---------------------\n 1 file changed, 67 insertions(+), 38 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex d726537622..00845cc03f 100644\n--- a/path.c\n+++ b/path.c\n@@ -1112,6 +1112,63 @@ const char *remove_leading_path(const char *in, const char *prefix)\n  * end with a '/', then the callers need to be fixed up accordingly.\n  *\n  */\n+\n+static const char *skip_slashes(const char *p)\n+{\n+\twhile (is_dir_sep(*p))\n+\t\tp++;\n+\treturn p;\n+}\n+\n+static int handle_dot_component(const char **src)\n+{\n+\tconst char *s = *src;\n+\n+\tif (*s != '.')\n+\t\treturn 0;\n+\n+\tif (!s[1]) {\n+\t\t*src = s + 1;\n+\t\treturn 1;\n+\t}\n+\n+\tif (is_dir_sep(s[1])) {\n+\t\t*src = skip_slashes(s + 2);\n+\t\treturn 1;\n+\t}\n+\n+\tif (s[1] == '.') {\n+\t\tif (!s[2]) {\n+\t\t\t*src = s + 2;\n+\t\t\treturn 2;\n+\t\t}\n+\t\tif (is_dir_sep(s[2])) {\n+\t\t\t*src = skip_slashes(s + 3);\n+\t\t\treturn 2;\n+\t\t}\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int strip_last_component(char **dst, char *dst0, int *prefix_len)\n+{\n+\tchar *d = *dst;\n+\n+\td--;\n+\tif (d <= dst0)\n+\t\treturn -1;\n+\n+\twhile (dst0 < d && d[-1] != '/')\n+\t\td--;\n+\n+\tif (prefix_len && *prefix_len > d - dst0)\n+\t\t*prefix_len = d - dst0;\n+\n+\t*dst = d;\n+\treturn 0;\n+}\n+\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n@@ -1129,8 +1186,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t}\n \tdst0 = dst;\n \n-\twhile (is_dir_sep(*src))\n-\t\tsrc++;\n+\tsrc = skip_slashes(src);\n \n \tfor (;;) {\n \t\tchar c = *src;\n@@ -1143,29 +1199,14 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t\t * (3) \"..\" and ends  -- strip one and terminate.\n \t\t * (4) \"../\"          -- strip one, eat slash and continue.\n \t\t */\n-\t\tif (c == '.') {\n-\t\t\tif (!src[1]) {\n-\t\t\t\t/* (1) */\n-\t\t\t\tsrc++;\n-\t\t\t} else if (is_dir_sep(src[1])) {\n-\t\t\t\t/* (2) */\n-\t\t\t\tsrc += 2;\n-\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\tsrc++;\n-\t\t\t\tcontinue;\n-\t\t\t} else if (src[1] == '.') {\n-\t\t\t\tif (!src[2]) {\n-\t\t\t\t\t/* (3) */\n-\t\t\t\t\tsrc += 2;\n-\t\t\t\t\tgoto up_one;\n-\t\t\t\t} else if (is_dir_sep(src[2])) {\n-\t\t\t\t\t/* (4) */\n-\t\t\t\t\tsrc += 3;\n-\t\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\t\tsrc++;\n-\t\t\t\t\tgoto up_one;\n-\t\t\t\t}\n-\t\t\t}\n+\t\tint dot = handle_dot_component(&src);\n+\n+\t\tif (dot == 1)\n+\t\t\tcontinue;\n+\t\tif (dot == 2) {\n+\t\t\tif (strip_last_component(&dst, dst0, prefix_len))\n+\t\t\t\treturn -1;\n+\t\t\tcontinue;\n \t\t}\n \n \t\t/* copy up to the next '/', and eat all '/' */\n@@ -1180,20 +1221,8 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t\t\tbreak;\n \t\tcontinue;\n \n-\tup_one:\n-\t\t/*\n-\t\t * dst0..dst is prefix portion, and dst[-1] is '/';\n-\t\t * go up one level.\n-\t\t */\n-\t\tdst--;\t/* go to trailing '/' */\n-\t\tif (dst <= dst0)\n-\t\t\treturn -1;\n-\t\t/* Windows: dst[-1] cannot be backslash anymore */\n-\t\twhile (dst0 < dst && dst[-1] != '/')\n-\t\t\tdst--;\n-\t\tif (prefix_len && *prefix_len > dst - dst0)\n-\t\t\t*prefix_len = dst - dst0;\n \t}\n+\n \t*dst = '\\0';\n \treturn 0;\n }\n-- \n2.43.0\n\n"},{"id":"534826","messageId":"xmqqh5s4b66w.fsf@gitster.g","threadId":"64883","inReplyTo":"20260129145434.29123-2-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH] path: refactor normalize_path_copy_len()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-29T18:54:31Z","receivedAt":"2026-01-29T18:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:\n\n> Refactor normalize_path_copy_len() by extracting helpers for skipping\n> slashes, handling dot components, and stripping the previous path\n> component, making the control flow easier to follow.\n\nThe new helper for skip_slashes() may be a clear win as it extracts\naway verbosity from 3 places.\n\nMoving the logic to the \"handle_dot_component()\" helper, however,\ndissociates the actual code from the explanation on the 4 special\ncases in the comment, and at least to me, made it a lot harder to\nunderstand what is being done and why.  Also the \"goto up_one\" logic\nwas easier to follow in the original than with the magic return\nvalues given by the new helper.  Quite honestly, use of that helper\nfunction looked like worsening the readablity of the logic.\n\nGiving a descriptive name to what is done at the up_one label by\nusing a single-shot helper function strip_last_component() may be an\nimprovement, but I do not think it is a clear win.  Adding a\nsingle-liner /* strip the last component */ comment without moving\nthe code may have made the result even easier to follow without\ndisrupting the flow with an extra helper function.\n\n path.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git i/path.c w/path.c\nindex d726537622..53a87ab67a 100644\n--- i/path.c\n+++ w/path.c\n@@ -1182,6 +1182,8 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \n \tup_one:\n \t\t/*\n+\t\t * strip the last component\n+\t\t *\n \t\t * dst0..dst is prefix portion, and dst[-1] is '/';\n \t\t * go up one level.\n \t\t */\n"},{"id":"534874","messageId":"20260130140143.5579-2-pushkarkumarsingh1970@gmail.com","threadId":"64883","inReplyTo":"xmqqh5s4b66w.fsf@gitster.g","subject":"[PATCH v2] path: factor out skip_slashes() in normalize_path_copy_len()","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-30T14:01:44Z","receivedAt":"2026-01-30T14:07:13Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Hi Junio,\n\nThanks for the detailed feedback.\n\nThis version keeps skip_slashes(), but drops handle_dot_component() and\nrestores the original control flow around the four dot-component cases.\nThe up_one logic is kept inline, with a short explanatory comment as\nsuggested.\n\nChanges since v1:\n  - Keep skip_slashes() helper.\n  - Restore inline dot-component handling.\n  - Remove handle_dot_component() helper.\n  - Keep up_one logic inline and add a brief comment.\n\nThanks for the review.\n\nPushkar\n---\n path.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex d726537622..1772fcb21c 100644\n--- a/path.c\n+++ b/path.c\n@@ -1112,6 +1112,14 @@ const char *remove_leading_path(const char *in, const char *prefix)\n  * end with a '/', then the callers need to be fixed up accordingly.\n  *\n  */\n+\n+static const char *skip_slashes(const char *p)\n+{\n+\twhile (is_dir_sep(*p))\n+\t\tp++;\n+\treturn p;\n+}\n+\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n@@ -1129,8 +1137,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t}\n \tdst0 = dst;\n \n-\twhile (is_dir_sep(*src))\n-\t\tsrc++;\n+\tsrc = skip_slashes(src);\n \n \tfor (;;) {\n \t\tchar c = *src;\n@@ -1150,8 +1157,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t\t\t} else if (is_dir_sep(src[1])) {\n \t\t\t\t/* (2) */\n \t\t\t\tsrc += 2;\n-\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\tsrc++;\n+\t\t\t\tsrc = skip_slashes(src);\n \t\t\t\tcontinue;\n \t\t\t} else if (src[1] == '.') {\n \t\t\t\tif (!src[2]) {\n@@ -1161,8 +1167,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t\t\t\t} else if (is_dir_sep(src[2])) {\n \t\t\t\t\t/* (4) */\n \t\t\t\t\tsrc += 3;\n-\t\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\t\tsrc++;\n+\t\t\t\t\tsrc = skip_slashes(src);\n \t\t\t\t\tgoto up_one;\n \t\t\t\t}\n \t\t\t}\n@@ -1182,6 +1187,8 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \n \tup_one:\n \t\t/*\n+\t\t * strip the last component\n+\t\t *\n \t\t * dst0..dst is prefix portion, and dst[-1] is '/';\n \t\t * go up one level.\n \t\t */\n-- \n2.43.0\n\n"},{"id":"536003","messageId":"20260214091406.15118-1-pushkarkumarsingh1970@gmail.com","threadId":"64883","inReplyTo":"20260130140143.5579-2-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v2] path: factor out skip_slashes() in normalize_path_copy_len()","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-14T09:13:58Z","receivedAt":"2026-02-14T09:14:13Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Hi Junio,\n\nJust checking back on the v2 in case it got missed.\nLet me know if you’d like me to tweak anything.\n\nThanks,\nPushkar\n"},{"id":"536213","messageId":"xmqqms17cjjj.fsf@gitster.g","threadId":"64883","inReplyTo":"20260214091406.15118-1-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v2] path: factor out skip_slashes() in normalize_path_copy_len()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-17T18:27:28Z","receivedAt":"2026-02-17T18:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:\n\n> Hi Junio,\n>\n> Just checking back on the v2 in case it got missed.\n> Let me know if you’d like me to tweak anything.\n>\n> Thanks,\n> Pushkar\n\nSorry, I saw it, I didn't think it was meant for application (it\ndidn't have a proper log message like v1 used to describve its\nchanges, which I expected to be updated to match the smaller scope\nof what v2 made---all it had was something akin to a cover letter)\nand did not comment on it when I saw it, and then completely forgot\nabout it.\n\n"},{"id":"536583","messageId":"20260221110511.1592-2-pushkarkumarsingh1970@gmail.com","threadId":"64883","inReplyTo":"xmqqms17cjjj.fsf@gitster.g","subject":"[PATCH v3] path: factor out skip_slashes() in normalize_path_copy_len()","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-02-21T11:05:12Z","receivedAt":"2026-02-21T11:06:50Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Extract skip_slashes() to avoid repeating the same is_dir_sep()\nloop in multiple places inside normalize_path_copy_len().\n\nKeep the dot-component handling inline to preserve the original\ncontrol flow and readability, as suggested in review.\n\nNo functional changes. Behavior verified with t0060-path-utils.sh.\n\nSigned-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>\n---\nChanges since v2:\n- Clarify commit message to reflect reduced scope.\n- Make intent explicit and ready for application.\n\n path.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex d726537622..1772fcb21c 100644\n--- a/path.c\n+++ b/path.c\n@@ -1112,6 +1112,14 @@ const char *remove_leading_path(const char *in, const char *prefix)\n  * end with a '/', then the callers need to be fixed up accordingly.\n  *\n  */\n+\n+static const char *skip_slashes(const char *p)\n+{\n+\twhile (is_dir_sep(*p))\n+\t\tp++;\n+\treturn p;\n+}\n+\n int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n {\n \tchar *dst0;\n@@ -1129,8 +1137,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t}\n \tdst0 = dst;\n \n-\twhile (is_dir_sep(*src))\n-\t\tsrc++;\n+\tsrc = skip_slashes(src);\n \n \tfor (;;) {\n \t\tchar c = *src;\n@@ -1150,8 +1157,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t\t\t} else if (is_dir_sep(src[1])) {\n \t\t\t\t/* (2) */\n \t\t\t\tsrc += 2;\n-\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\tsrc++;\n+\t\t\t\tsrc = skip_slashes(src);\n \t\t\t\tcontinue;\n \t\t\t} else if (src[1] == '.') {\n \t\t\t\tif (!src[2]) {\n@@ -1161,8 +1167,7 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \t\t\t\t} else if (is_dir_sep(src[2])) {\n \t\t\t\t\t/* (4) */\n \t\t\t\t\tsrc += 3;\n-\t\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\t\tsrc++;\n+\t\t\t\t\tsrc = skip_slashes(src);\n \t\t\t\t\tgoto up_one;\n \t\t\t\t}\n \t\t\t}\n@@ -1182,6 +1187,8 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len)\n \n \tup_one:\n \t\t/*\n+\t\t * strip the last component\n+\t\t *\n \t\t * dst0..dst is prefix portion, and dst[-1] is '/';\n \t\t * go up one level.\n \t\t */\n-- \n2.43.0\n\n"}]}