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

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

[Clemens Buchacher]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

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:
> 
> @@ -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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

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

[Nguyễn Thái Ngọc Duy]

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(-)

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