Re: [PATCH] path: refactor normalize_path_copy_len()

[Junio C Hamano <gitster@pobox.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.
 		 */