Re: [PATCH] get_cwd_relative(): do not misinterpret root path

Clemens Buchacher· Nov 19, 2010, 20:41 UTC · lore

Hi Duy,

Thanks for looking into this.

On Fri, Nov 19, 2010 at 04:17:19PM +0700, Nguyễn Thái Ngọc Duy wrote:

Show 14 quoted lines

> 
> @@ -965,6 +965,12 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)
>  	case '/':
>  		return cwd + 1;
>  	default:
> +		/*
> +		 * dir can end with a path separator when it's root
> +		 * directory. Return proper prefix in that case.
> +		 */
> +		if (is_dir_sep(dir[-1]))
> +			return cwd;
>  		return NULL;
>  	}
>  }

make_absolute_path() already rewrites dir to contain slashes only. I think it is confusing to use is_dir_sep() here, when we only check for / above.

Otherwise, the patch looks good to me.

Clemens

Nguyễn Thái Ngọc Duy· Nov 20, 2010, 06:50 UTC · re: Clemens Buchacher · lore

[PATCH v2] get_cwd_relative(): do not misinterpret root path

Commit 490544b (get_cwd_relative(): do not misinterpret suffix as subdirectory) handles case where:

dir = "/path/work"; cwd = "/path/work-xyz";

When it comes to the end of get_cwd_relative(), dir is at '\0' and cwd is at '-'. The rest of cwd, "-xyz", clearly cannot be the relative path from dir to cwd. However there is another case where:

dir = "/"; /* or even "c:/" */ cwd = "/path/to/here";

In this special case, while *cwd == 'p', which is not a path separator, the rest of cwd, "path/to/here", can be returned as a relative path from dir to cwd.

Handle this case and make t1509 pass again.

Reported-by: Albert Strasheim <fullung@gmail.com>
Reported-by: Matthijs Kooijman <matthijs@stdin.nl>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 2010/11/20 Clemens Buchacher <drizzd@aon.at>:
 > make_absolute_path() already rewrites dir to contain slashes only.
 > I think it is confusing to use is_dir_sep() here, when we only
 > check for / above.

 I was thinking about GIT_WORK_TREE="c:\" (so dir would be "c:\") when
 I wrote that. It is recognized as absolute path, no normalization
 will be done.

 But then the comparison part earlier in this function would fail because
 getcwd in Windows port always return '/'.

 I will make a patch to convert GIT_WORK_TREE (and GIT_DIR too?) to '/'
 so everything is consistent.

 dir.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

Show changes to dir.c +6 −0

diff --git a/dir.c b/dir.c
index b2dfb69..b687504 100644
--- a/dir.c
+++ b/dir.c
@@ -965,6 +965,12 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)
 	case '/':
 		return cwd + 1;
 	default:
+		/*
+		 * dir can end with a path separator when it's root
+		 * directory. Return proper prefix in that case.
+		 */
+		if (dir[-1] == '/')
+			return cwd;
 		return NULL;
 	}
 }

-- 
1.7.3.2.316.gda8b3

patchRe: [PATCH] get_cwd_relative(): do not misinterpret root path