Martin Erik Werner <martinerikwer...@gmail.com> writes:

> In order to manipulate symliks in the
> work tree using absolute paths, symlinks should only be dereferenced
> outside the work tree.

I agree 100% with this reasoning (modulo s/symliks/symlinks/).

As to the implementation, it looks a bit overly complicated,
though.  I haven't tried writing the same myself, but 

 * I suspect that strbuf would help simplifying the allocation and
   deallocation;

 * Also I suspect that use of string-list to split and then join is
   making the code unnecessarily complex. In Python/Perl, that would
   be a normal approach, but in C, it would be a lot simpler if you
   prepare a writable temporary in wtpart[], walk from left to right
   finding '/' and replacing it temporarily with NUL to terminate in
   order to check with real_path(), restore the NUL back to '/' to
   check deeper, and rinse and repeat.

   Having said that, I am not absolutely sure if the repeated
   calls to real_path() are doing the right thing, though ;-)

> diff --git a/setup.c b/setup.c
> index 5432a31..0789a96 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -22,11 +22,51 @@ char *prefix_path_gently(const char *prefix, int len,
>       const char *orig = path;
>       char *sanitized;
>       if (is_absolute_path(orig)) {
> -             const char *temp = real_path(path);
> -             sanitized = xmalloc(len + strlen(temp) + 1);
> -             strcpy(sanitized, temp);
> +             int i, match;
> +             size_t wtpartlen;
> +             char *npath, *wtpart;
> +             struct string_list list = STRING_LIST_INIT_DUP;
> +             const char *work_tree = get_git_work_tree();
> +             if (!work_tree)
> +                     return NULL;
> +             npath = xmalloc(strlen(path) + 1);
>               if (remaining_prefix)
>                       *remaining_prefix = 0;
> +             if (normalize_path_copy_len(npath, path, remaining_prefix)) {
> +                     free(npath);
> +                     return NULL;
> +             }
> +
> +             string_list_split(&list, npath, '/', -1);
> +             wtpart = xmalloc(strlen(npath) + 1);
> +             i = 0;
> +             match = 0;
> +             strcpy(wtpart, list.items[i++].string);
> +             strcat(wtpart, "/");
> +             if (strcmp(real_path(wtpart), work_tree) == 0) {
> +                     match = 1;
> +             } else {
> +                     while (i < list.nr) {
> +                             strcat(wtpart, list.items[i++].string);
> +                             if (strcmp(real_path(wtpart), work_tree) == 0) {
> +                                     match = 1;
> +                                     break;
> +                             }
> +                             strcat(wtpart, "/");
> +                     }
> +             }
> +             string_list_clear(&list, 0);
> +             if (!match) {
> +                     free(npath);
> +                     free(wtpart);
> +                     return NULL;
> +             }
> +
> +             wtpartlen = strlen(wtpart);
> +             sanitized = xmalloc(strlen(npath) - wtpartlen);
> +             strcpy(sanitized, npath + wtpartlen + 1);
> +             free(npath);
> +             free(wtpart);
>       } else {
>               sanitized = xmalloc(len + strlen(path) + 1);
>               if (len)
> @@ -34,26 +74,10 @@ char *prefix_path_gently(const char *prefix, int len,
>               strcpy(sanitized + len, path);
>               if (remaining_prefix)
>                       *remaining_prefix = len;
> -     }
> -     if (normalize_path_copy_len(sanitized, sanitized, remaining_prefix))
> -             goto error_out;
> -     if (is_absolute_path(orig)) {
> -             size_t root_len, len, total;
> -             const char *work_tree = get_git_work_tree();
> -             if (!work_tree)
> -                     goto error_out;
> -             len = strlen(work_tree);
> -             root_len = offset_1st_component(work_tree);
> -             total = strlen(sanitized) + 1;
> -             if (strncmp(sanitized, work_tree, len) ||
> -                 (len > root_len && sanitized[len] != '\0' && sanitized[len] 
> != '/')) {
> -             error_out:
> +             if (normalize_path_copy_len(sanitized, sanitized, 
> remaining_prefix)) {
>                       free(sanitized);
>                       return NULL;
>               }
> -             if (sanitized[len] == '/')
> -                     len++;
> -             memmove(sanitized, sanitized + len, total - len);
>       }
>       return sanitized;
>  }
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 3a0677a..03a12ac 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -190,7 +190,7 @@ test_expect_success SYMLINKS 'real path works on 
> symlinks' '
>       test "$sym" = "$(test-path-utils real_path "$dir2/syml")"
>  '
>  
> -test_expect_failure SYMLINKS 'prefix_path works with work tree symlinks' '
> +test_expect_success SYMLINKS 'prefix_path works with work tree symlinks' '
>  
>       ln -s target symlink &&
>       test "$(test-path-utils prefix_path prefix "$(pwd)/symlink")" = 
> "symlink"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to