Re: [PATCH v4 3/4] setup: Add 'abspath_part_inside_repo' function

On Sun, Feb 02, 2014 at 12:37:16PM +0100, Torsten Bögershausen wrote:

On 2014-02-02 12.21, David Kastrup wrote:

quoted

Martin Erik Werner [off-list ref] writes:

quoted

On Sun, Feb 02, 2014 at 09:19:04AM +0700, Duy Nguyen wrote:

quoted

On Sun, Feb 2, 2014 at 8:59 AM, Martin Erik Werner
[off-list ref] wrote:

quoted

+       /* check if work tree is already the prefix */
+       if (strncmp(path, work_tree, wtlen) == 0) {
+               if (path[wtlen] == '/')
+                       memmove(path, path + wtlen + 1, len - wtlen);
+               else
+                       /* work tree is the root, or the whole path */
+                       memmove(path, path + wtlen, len - wtlen + 1);
+               return 0;
+       }

No the 4th time is not the charm yet :) if path is "/abc/defghi" and
work_tree is "/abc/def" you don't want to return "ghi" as the prefix
here.

Ah indeed, this should catch that:

diff --git a/setup.c b/setup.c
index 2270bd4..5817875 100644
--- a/setup.c
+++ b/setup.c

@@ -32,9 +32,11 @@ static inline int abspath_part_inside_repo(char *path)
 	if (strncmp(path, work_tree, wtlen) == 0) {
 		if (path[wtlen] == '/')
 			memmove(path, path + wtlen + 1, len - wtlen);
-		else
+		else if (path[wtlen - 1] == '/' || path[wtlen] == '\0')

Is wtlen guaranteed to be nonzero?

Hmm, am I incorrect in thinking
if (!work_tree)
takes care of that?

Another comment:
The "raw" comparison with '/' is probably working well on all
POSIX/Linux/Unix systems.

To be more portable, the macro
is_dir_sep()
can be used:

if (is_dir_sep(path[wtlen]))

Since the path is already normalized by 'normalize_path_copy_len' which
seems to guarantee '/'-separation, I have assumed that this was
unnecessary?

--
Martin Erik Werner [off-list ref]