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 martinerikwer...@gmail.com
---
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 martinerikwer...@gmail.com
---
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 martinerikwer...@gmail.com
--
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
martinerikwer...@gmail.com 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
