[PATCH v3 3/4] setup: Add 'abspath_part_inside_repo' function
In order to extract the part of an absolute path which lies inside the repo, it is not possible to directly use real_path, since that would dereference symlinks both outside and inside the work tree. Add an 'abspath_part_inside_repo' function which incrementally checks each path level by temporarily NUL-terminating at each '/' and comparing against the work tree path. When a match is found, it copies the remainder (which will be the in-repo part) to a destination buffer. The path being the filesystem root or exactly equal to the work tree are special cases handled separately, since then there is no directory separator between the work tree and in-repo part. Signed-off-by: Martin Erik Werner --- cache.h | 1 + setup.c | 63 +++ 2 files changed, 64 insertions(+) diff --git a/cache.h b/cache.h index ce377e1..242f27d 100644 --- a/cache.h +++ b/cache.h @@ -426,6 +426,7 @@ extern void verify_filename(const char *prefix, int diagnose_misspelt_rev); extern void verify_non_filename(const char *prefix, const char *name); extern int path_inside_repo(const char *prefix, const char *path); +extern int abspath_part_inside_repo(char *dst, const char *path); #define INIT_DB_QUIET 0x0001 diff --git a/setup.c b/setup.c index 5432a31..e606846 100644 --- a/setup.c +++ b/setup.c @@ -77,6 +77,69 @@ int path_inside_repo(const char *prefix, const char *path) return 0; } +/* + * It is ok if dst == src, but they should not overlap otherwise. + * No checking if the path is in fact an absolute path is done, and it must + * already be normalized. + * + * Find the part of an absolute path that lies inside the work tree by + * dereferencing symlinks outside the work tree, for example: + * /dir1/repo/dir2/file (work tree is /dir1/repo) -> dir2/file + * /dir/file (work tree is /) -> dir/file + * /dir/symlink1/symlink2 (symlink1 points to work tree) -> symlink2 + * /dir/repo (exactly equal to work tree) -> (empty string) + */ +int abspath_part_inside_repo(char *dst, const char* src) +{ + size_t len; + char *dst0; + char temp; + + const char* work_tree = get_git_work_tree(); + if (!work_tree) + return -1; + len = strlen(src); + dst0 = dst; + + // check root level + if (has_dos_drive_prefix(src)) { + *dst++ = *src++; + *dst++ = *src++; + *dst++ = *src++; + } else { + *dst++ = *src++; + } + temp = *dst; + *dst = '\0'; + if (strcmp(real_path(dst0), work_tree) == 0) { + *dst = temp; + memmove(dst0, src, len - (dst - dst0) + 1); + return 0; + } + *dst = temp; + + // check each level + while (*dst != '\0') { + *dst++ = *src++; + if (*dst == '/') { + *dst = '\0'; + if (strcmp(real_path(dst0), work_tree) == 0) { + memmove(dst0, src + 1, len - (dst - dst0)); + return 0; + } + *dst = '/'; + } + } + + // check whole path + if (strcmp(real_path(dst0), work_tree) == 0) { + *dst0 = '\0'; + return 0; + } + + return -1; +} + int check_filename(const char *prefix, const char *arg) { const char *name; -- 1.8.5.2 -- 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
Re: [PATCH v3 3/4] setup: Add 'abspath_part_inside_repo' function
On 2014-01-31 21.22, Martin Erik Werner wrote: > In order to extract the part of an absolute path which lies inside the > repo, it is not possible to directly use real_path, since that would > dereference symlinks both outside and inside the work tree. > > Add an 'abspath_part_inside_repo' function which incrementally checks > each path level by temporarily NUL-terminating at each '/' and comparing > against the work tree path. When a match is found, it copies the > remainder (which will be the in-repo part) to a destination > buffer. > > The path being the filesystem root or exactly equal to the work tree are > special cases handled separately, since then there is no directory > separator between the work tree and in-repo part. > > Signed-off-by: Martin Erik Werner > --- > cache.h | 1 + > setup.c | 63 +++ > 2 files changed, 64 insertions(+) > > diff --git a/cache.h b/cache.h > index ce377e1..242f27d 100644 > --- a/cache.h > +++ b/cache.h > @@ -426,6 +426,7 @@ extern void verify_filename(const char *prefix, > int diagnose_misspelt_rev); > extern void verify_non_filename(const char *prefix, const char *name); > extern int path_inside_repo(const char *prefix, const char *path); > +extern int abspath_part_inside_repo(char *dst, const char *path); abspath_part_inside_repo() is only used in setup.c, isn't it? In this case we don't need it in cache.h, it can be declared inside setup.c as static int abspath_part_inside_repo(char *dst, const char *path); (or "static inline" ) - (And not in this patch: see the final setup.c:) if (g) { free(npath); return NULL; } If this is the only caller of abspath_part_inside_repo(), then do we need npath 2 times as a parameter ? Or can we re-write it to look like this: static inline int abspath_part_inside_repo(char *path) [ ] -- 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
Re: [PATCH v3 3/4] setup: Add 'abspath_part_inside_repo' function
On Fri, Jan 31, 2014 at 11:37:29PM +0100, Torsten Bögershausen wrote: > On 2014-01-31 21.22, Martin Erik Werner wrote: > > In order to extract the part of an absolute path which lies inside the > > repo, it is not possible to directly use real_path, since that would > > dereference symlinks both outside and inside the work tree. > > > > Add an 'abspath_part_inside_repo' function which incrementally checks > > each path level by temporarily NUL-terminating at each '/' and comparing > > against the work tree path. When a match is found, it copies the > > remainder (which will be the in-repo part) to a destination > > buffer. > > > > The path being the filesystem root or exactly equal to the work tree are > > special cases handled separately, since then there is no directory > > separator between the work tree and in-repo part. > > > > Signed-off-by: Martin Erik Werner > > --- > > cache.h | 1 + > > setup.c | 63 > > +++ > > 2 files changed, 64 insertions(+) > > > > diff --git a/cache.h b/cache.h > > index ce377e1..242f27d 100644 > > --- a/cache.h > > +++ b/cache.h > > @@ -426,6 +426,7 @@ extern void verify_filename(const char *prefix, > > int diagnose_misspelt_rev); > > extern void verify_non_filename(const char *prefix, const char *name); > > extern int path_inside_repo(const char *prefix, const char *path); > > +extern int abspath_part_inside_repo(char *dst, const char *path); > abspath_part_inside_repo() is only used in setup.c, isn't it? > In this case we don't need it in cache.h, it can be declared inside setup.c as > > static int abspath_part_inside_repo(char *dst, const char *path); > (or "static inline" ) > > - > (And not in this patch: see the final setup.c:) > > if (g) { > free(npath); > return NULL; > } > > If this is the only caller of abspath_part_inside_repo(), > then do we need npath 2 times as a parameter ? > Or can we re-write it to look like this: > > static inline int abspath_part_inside_repo(char *path) > [ > ] I guess I've over-generalised it a bit too much, that should rather be done if-and-when, I presume? It is indeed only used in setup.c and only by the prefix_path_gently function so static inline then? Hmm, for single-parameter it should suffice to simply move the parameter down into the function, like so?: const char* src; src = dst; and carry on as before (obviously also renaming the variables sensibly), or did you have something else in mind? (I added two parameters since I was glancing at 'normalize_path_copy_len' for inspiration, and was thinking about (purely theoretical) re-use in other cases rather than minimizing it for the time being.) What do you mean with the "(And not in this patch"... bit; what "final setup.c"? -- Martin Erik Werner -- 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
Re: [PATCH v3 3/4] setup: Add 'abspath_part_inside_repo' function
On Sat, Feb 1, 2014 at 3:22 AM, Martin Erik Werner wrote: > diff --git a/setup.c b/setup.c > index 5432a31..e606846 100644 > --- a/setup.c > +++ b/setup.c > @@ -77,6 +77,69 @@ int path_inside_repo(const char *prefix, const char *path) > return 0; > } > > +/* > + * It is ok if dst == src, but they should not overlap otherwise. > + * No checking if the path is in fact an absolute path is done, and it must > + * already be normalized. > + * > + * Find the part of an absolute path that lies inside the work tree by > + * dereferencing symlinks outside the work tree, for example: > + * /dir1/repo/dir2/file (work tree is /dir1/repo) -> dir2/file > + * /dir/file (work tree is /) -> dir/file > + * /dir/symlink1/symlink2 (symlink1 points to work tree) -> symlink2 > + * /dir/repo (exactly equal to work tree) -> (empty string) > + */ > +int abspath_part_inside_repo(char *dst, const char* src) > +{ > + size_t len; > + char *dst0; > + char temp; > + > + const char* work_tree = get_git_work_tree(); > + if (!work_tree) > + return -1; > + len = strlen(src); > + dst0 = dst; > + > + // check root level Um.. no C++ style comments. And there should be a test that work_tree is the prefix of src (common case). If so we can return early and do not need to do real_path() on every path component. > + if (has_dos_drive_prefix(src)) { > + *dst++ = *src++; > + *dst++ = *src++; > + *dst++ = *src++; > + } else { > + *dst++ = *src++; > + } > + temp = *dst; > + *dst = '\0'; > + if (strcmp(real_path(dst0), work_tree) == 0) { > + *dst = temp; > + memmove(dst0, src, len - (dst - dst0) + 1); > + return 0; > + } > + *dst = temp; > + > + // check each level > + while (*dst != '\0') { > + *dst++ = *src++; > + if (*dst == '/') { > + *dst = '\0'; > + if (strcmp(real_path(dst0), work_tree) == 0) { > + memmove(dst0, src + 1, len - (dst - dst0)); > + return 0; > + } > + *dst = '/'; > + } > + } > + > + // check whole path > + if (strcmp(real_path(dst0), work_tree) == 0) { > + *dst0 = '\0'; > + return 0; > + } > + > + return -1; > +} > + > int check_filename(const char *prefix, const char *arg) > { > const char *name; > -- > 1.8.5.2 -- Duy -- 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