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

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

+       /* 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.

+       path0 = path;
+       path += offset_1st_component(path);
+
+       /* check each level */
+       while (*path != '\0') {
+               path++;

To me it looks like we could write

for (; *path; path++) {

or even

for (path += offset_1st_component(path); *path; path++) {

but it's personal taste..

+               if (*path == '/') {
+                       *path = '\0';
+                       if (strcmp(real_path(path0), work_tree) == 0) {
+                               memmove(path0, path + 1, len - (path - path0));
+                               return 0;
+                       }
+                       *path = '/';
+               }
+       }
+
+       /* check whole path */
+       if (strcmp(real_path(path0), work_tree) == 0) {
+               *path0 = '\0';
+               return 0;
+       }

I think this is already handled by the "check if work tree is already
the prefix" block.

+
+       return -1;
+}
+
+/*
  * Normalize "path", prepending the "prefix" for relative paths. If
  * remaining_prefix is not NULL, return the actual prefix still
  * remains in the path. For example, prefix = sub1/sub2/ and path is
--
1.8.5.2

-- 
Duy