On Sun, Feb 02, 2014 at 09:19:04AM +0700, Duy Nguyen wrote:
> On Sun, Feb 2, 2014 at 8:59 AM, Martin Erik Werner
> <martinerikwerner@xxxxxxxxx> 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.
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')
/* work tree is the root, or the whole path */
memmove(path, path + wtlen, len - wtlen + 1);
+ else
+ return -1;
return 0;
}
path0 = path;
Is it worth adding a test for this as well?:
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index f6f378b..05d3366 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -201,6 +201,10 @@ test_expect_success 'prefix_path works with only absolute path to work tree' '
test_cmp expected actual
'
+test_expect_success 'prefix_path rejects absolute path to dir with same beginning as work tree' '
+ test_must_fail test-path-utils prefix_path prefix "$(pwd)a"
+'
+
relative_path /foo/a/b/c/ /foo/a/b/ c/
relative_path /foo/a/b/c/ /foo/a/b c/
relative_path /foo/a//b//c/ ///foo/a/b// c/ POSIX
> > + 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..
Yeah, I think aesthetically I don't like cramming too much into the for loop:
for (path += offset_1st_component(path0) + 1; *path; path++) {
neither leaving the init expression unused. So as long as it's just personal
taste I think I'll stick with the current while loop format. But I'll exchange
(*path == '\0') for (*path) though.
--
Martin Erik Werner <martinerikwerner@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html