Re: [PATCH 2/2] setup: Don't dereference in-tree symlinks for absolute paths

On Sun, Jan 26, 2014 at 06:19:25PM +0100, Torsten Bögershausen wrote:

On 2014-01-26 15.22, Martin Erik Werner wrote:

quoted

The prefix_path_gently() function currently applies real_path to
everything if given an absolute path, dereferencing symlinks both
outside and inside the work tree. In order to manipulate symliks in the
work tree using absolute paths, symlinks should only be dereferenced
outside the work tree.

Modify prefix_path_gently() to first normalize the path in order to
make sure path levels are separated by '/', then use this separator to
check the real path of each level of the path until it has found the
length that corresponds to the work tree.

For absolute paths, the function did not, nor does now do, any actual
prefixing, hence we simply remove the path corresponding to the work
tree and return the remaining in-tree part of the path.

Fixes t0060-82.

Signed-off-by: Martin Erik Werner <redacted>
---
 setup.c               | 54 ++++++++++++++++++++++++++++++++-------------------
 t/t0060-path-utils.sh |  2 +-
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/setup.c b/setup.c
index 6c3f85f..bec587e 100644
--- a/setup.c
+++ b/setup.c

@@ -22,11 +22,41 @@ 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);
+		char npath[strlen(path)];

Is this portable ?
This is variable-length array, isn't it ?
Using xmalloc() may be better

Ah, right, that looks bad now that you mention it.

quoted

 		if (remaining_prefix)
 			*remaining_prefix = 0;
+		if (normalize_path_copy_len(npath, path, remaining_prefix))
+			return NULL;
+		const char *work_tree = get_git_work_tree();

declaration after statements should be avoided (not only here)

Indeed, I somehow guessed that declaration-after-statement was ok and
tried to keep them close to usage, bad guess, evidently.

I've rerolled the last v1 patch accordingly:
* Use xmalloc() when initializing char* from strlen()
* Separate and move declarations to beginning of scope
* Fix a strcpy that should've been a strcat (which would've nuked DOS
  prefixes, I think)

  [PATCH v2 2/2] setup: Don't dereference in-tree symlinks for absolute paths

 setup.c               | 64 +++++++++++++++++++++++++++++++++++----------------
 t/t0060-path-utils.sh |  2 +-
 2 files changed, 45 insertions(+), 21 deletions(-)

--
Martin Erik Werner [off-list ref]