Re: [PATCH] path: refactor normalize_path_copy_len()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 29, 2026, 18:54 UTC
- Message-ID
- <xmqqh5s4b66w.fsf@gitster.g>
- In-Reply-To
- <20260129145434.29123-2-pushkarkumarsingh1970@gmail.com>
Pushkar Singh <pushkarkumarsingh1970@gmail.com> writes:
> Refactor normalize_path_copy_len() by extracting helpers for skipping > slashes, handling dot components, and stripping the previous path > component, making the control flow easier to follow.
The new helper for skip_slashes() may be a clear win as it extracts away verbosity from 3 places.
Moving the logic to the "handle_dot_component()" helper, however, dissociates the actual code from the explanation on the 4 special cases in the comment, and at least to me, made it a lot harder to understand what is being done and why. Also the "goto up_one" logic was easier to follow in the original than with the magic return values given by the new helper. Quite honestly, use of that helper function looked like worsening the readablity of the logic.
Giving a descriptive name to what is done at the up_one label by using a single-shot helper function strip_last_component() may be an improvement, but I do not think it is a clear win. Adding a single-liner /* strip the last component */ comment without moving the code may have made the result even easier to follow without disrupting the flow with an extra helper function.
path.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git i/path.c w/path.c index d726537622..53a87ab67a 100644 --- i/path.c +++ w/path.c @@ -1182,6 +1182,8 @@ int normalize_path_copy_len(char *dst, const char *src, int *prefix_len) up_one: /* + * strip the last component + * * dst0..dst is prefix portion, and dst[-1] is '/'; * go up one level. */