threads / patch / 22589

patch, 4 partsmake_absolute_path(): Do not append redundant slash

Subject: [PATCH 1/4] make_absolute_path(): Do not append redundant slash

## tl;dr

12 messages between Feb 9, 2010 and Feb 11, 2010. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Feb 9, 2010, 17:02 UTC · lore

make_absolute_path("foo") at root returns "//foo". This patch makes it return "/foo" correctly.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 .. so that "git init" will show "initialized empty Git in /.git"
 abspath.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
Show changes to abspath.c +3 −2
diff --git a/abspath.c b/abspath.c
index b88122c..e72aede 100644
--- a/abspath.c
+++ b/abspath.c
@@ -54,8 +54,9 @@ const char *make_absolute_path(const char *path)
 			if (len + strlen(last_elem) + 2 > PATH_MAX)
 				die ("Too long path name: '%s/%s'",
 						buf, last_elem);
-			buf[len] = '/';
-			strcpy(buf + len + 1, last_elem);
+			if (*buf != '/' || buf[1] != '\0')
+				buf[len++] = '/';
+			strcpy(buf + len, last_elem);
 			free(last_elem);
 			last_elem = NULL;
 		}
-- 
1.7.0.rc2.182.g3adef
Nguyễn Thái Ngọc Duy· Feb 9, 2010, 17:02 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 2/4] rev-parse: make --git-dir return /.git instead of //.git

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin-rev-parse.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin-rev-parse.c +1 −1
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index a8c5043..9c22cd1 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -647,7 +647,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				}
 				if (!getcwd(cwd, PATH_MAX))
 					die_errno("unable to get current working directory");
-				printf("%s/.git\n", cwd);
+				printf("%s%s.git\n", cwd, *cwd == '/' && cwd[1] == '\0' ? "" : "/");
 				continue;
 			}
 			if (!strcmp(arg, "--is-inside-git-dir")) {
-- 
1.7.0.rc2.182.g3adef
Johannes Sixt· Feb 9, 2010, 19:18 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 2/4] rev-parse: make --git-dir return /.git instead of //.git

On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
Show 6 quoted lines
> @@ -647,7 +647,7 @@ int cmd_rev_parse(int argc, const char **argv, const
> char *prefix) }
>  				if (!getcwd(cwd, PATH_MAX))
>  					die_errno("unable to get current working directory");
> -				printf("%s/.git\n", cwd);
> +				printf("%s%s.git\n", cwd, *cwd == '/' && cwd[1] == '\0' ? "" : "/");

On Windows, when you are in the root of a drive, then cwd is "C:/", i.e. there is a trailing slash just as in the Unix root directory. But you do not take care of this situation. That is, you would print "C://".

How about:
static inline int is_root_path(const char *path)
{
	if (has_dos_drive_prefix(path))
		path += 2;
	while (is_dir_sep(*path))
		path++;
	return !*path;
}
and use it though-out your series?

(Simplify the loop to 'return is_dir_sep(*path) && !path[1];' if you can assume that paths are nomalized.)

-- Hannes
Nguyen Thai Ngoc Duy· Feb 11, 2010, 10:07 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/4] rev-parse: make --git-dir return /.git instead of //.git

On Wed, Feb 10, 2010 at 2:18 AM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 27 quoted lines
> On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>> @@ -647,7 +647,7 @@ int cmd_rev_parse(int argc, const char **argv, const
>> char *prefix) }
>>                               if (!getcwd(cwd, PATH_MAX))
>>                                       die_errno("unable to get current working directory");
>> -                             printf("%s/.git\n", cwd);
>> +                             printf("%s%s.git\n", cwd, *cwd == '/' && cwd[1] == '\0' ? "" : "/");
>
> On Windows, when you are in the root of a drive, then cwd is "C:/", i.e. there
> is a trailing slash just as in the Unix root directory. But you do not take
> care of this situation. That is, you would print "C://".
>
> How about:
>
> static inline int is_root_path(const char *path)
> {
>        if (has_dos_drive_prefix(path))
>                path += 2;
>        while (is_dir_sep(*path))
>                path++;
>        return !*path;
> }
>
> and use it though-out your series?
>
> (Simplify the loop to 'return is_dir_sep(*path) && !path[1];' if you can
> assume that paths are nomalized.)

And return the length of root_path, so that I can use this function in in setup_git_directory_gently() too. Yeah.

-- 
Duy
Nguyễn Thái Ngọc Duy· Feb 9, 2010, 17:02 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 3/4] Support working directory located at root

Git should work regardless where the working directory is located, even at root. This patch fixes two places where it assumes working directory always have parent directory.

In setup_git_directory_gently(), when Git goes up to root and finds .git there, it happily sets worktree to "" instead of "/".

In prefix_path(), loosen the outside repo check a little bit. Usually when a path XXX is inside worktree /foo, it must be either "/foo", or "/foo/...". When worktree is simply "/", we can safely ignore the check: we have a slash at the beginning already.

Not related to worktree, but also set gitdir correctly if a bare repo is placed (insanely?) at root.

Thanks João Carlos Mendes Luís for pointing out this problem.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I said I would have code change for DOS drive too. But I take it back.
 Supporting GIT_DIR=C:\.git might be easy, GIT_DIR=C:.git is not.
 setup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Show changes to setup.c +3 −3
diff --git a/setup.c b/setup.c
index b38cbee..4d24a74 100644
--- a/setup.c
+++ b/setup.c
@@ -25,7 +25,7 @@ const char *prefix_path(const char *prefix, int len, const char *path)
 		len = strlen(work_tree);
 		total = strlen(sanitized) + 1;
 		if (strncmp(sanitized, work_tree, len) ||
-		    (sanitized[len] != '\0' && sanitized[len] != '/')) {
+		    (len > 1 && sanitized[len] != '\0' && sanitized[len] != '/')) {
 		error_out:
 			die("'%s' is outside repository", orig);
 		}
@@ -403,7 +403,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			if (!work_tree_env)
 				inside_work_tree = 0;
 			if (offset != len) {
-				cwd[offset] = '\0';
+				cwd[offset ? offset : 1] = '\0';
 				set_git_dir(cwd);
 			} else
 				set_git_dir(".");
@@ -427,7 +427,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	inside_git_dir = 0;
 	if (!work_tree_env)
 		inside_work_tree = 1;
-	git_work_tree_cfg = xstrndup(cwd, offset);
+	git_work_tree_cfg = xstrndup(cwd, offset ? offset : 1);
 	if (check_repository_format_gently(nongit_ok))
 		return NULL;
 	if (offset == len)
-- 
1.7.0.rc2.182.g3adef
Johannes Sixt· Feb 9, 2010, 19:19 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 3/4] Support working directory located at root

On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>  I said I would have code change for DOS drive too. But I take it back.
>  Supporting GIT_DIR=C:\.git might be easy, GIT_DIR=C:.git is not.

One does not set GIT_DIR=C:.git; it would be insane because it means ".git in an unpredictable directory somewhere on drive C". It would be great to support GIT_DIR=C:/.git

-- Hannes
Nguyen Thai Ngoc Duy· Feb 11, 2010, 12:44 UTC · re: Johannes Sixt · lore

Re: [PATCH 3/4] Support working directory located at root

On 2/10/10, Johannes Sixt <j6t@kdbg.org> wrote:
Show 8 quoted lines
> On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>  >  I said I would have code change for DOS drive too. But I take it back.
>  >  Supporting GIT_DIR=C:\.git might be easy, GIT_DIR=C:.git is not.
>
>
> One does not set GIT_DIR=C:.git; it would be insane because it means ".git in
>  an unpredictable directory somewhere on drive C". It would be great to
>  support GIT_DIR=C:/.git

A bit off topic, but make_relative_path() may need more care for the Windows port. I thought of making relative path between C:/foo and D:/bar but it should work well for that case. //machine/share1/foo and //machine/share2/bar may fail though.

-- 
Duy
Nguyễn Thái Ngọc Duy· Feb 9, 2010, 17:02 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 4/4] Add test for using Git at root of file system

This kind of test requires a throw-away root filesystem so that it can play on. If you have such a system, go ahead, "chmod 777 /" and run this test manually ("make test" with root permission won't work).

If you don't but have Linux, static linked busybox, rsync and root permission, then run "t/t1509/prepare-chroot.sh /tmp/test". You will be instructed to create /dev/null and given a command to chroot into. Within chroot, you will be placed at directory "t" of Git source code Feel free to burn your chroot.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 May have problem committing. Anyway I don't need commit now.
 t/t1509-root-worktree.sh  |  244 +++++++++++++++++++++++++++++++++++++++++++++
 t/t1509/excludes          |   14 +++
 t/t1509/prepare-chroot.sh |   38 +++++++
 3 files changed, 296 insertions(+), 0 deletions(-)
 create mode 100755 t/t1509-root-worktree.sh
 create mode 100644 t/t1509/excludes
 create mode 100755 t/t1509/prepare-chroot.sh
Show changes to 3 files +296 −0

t/t1509-root-worktree.sh, t/t1509/excludes, t/t1509/prepare-chroot.sh

diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh
new file mode 100755
index 0000000..ae01b6d
--- /dev/null
+++ b/t/t1509-root-worktree.sh
@@ -0,0 +1,244 @@
+#!/bin/sh
+
+test_description='Test Git when git repository is located at root
+
+This test requires write access in root. Do not bother if you do not
+have a throwaway chroot or VM.
+
+Script t1509/prepare-chroot.sh may help you setup chroot, then you
+can chroot in and execute this test from there.
+'
+
+. ./test-lib.sh
+
+test_cmp_val() {
+	echo "$1" > expected
+	echo "$2" > result
+	test_cmp expected result
+}
+
+test_vars() {
+	test_expect_success "$1: gitdir" '
+		test_cmp_val "'"$2"'" "$(git rev-parse --git-dir)"
+	'
+
+	test_expect_success "$1: worktree" '
+		test_cmp_val "'"$3"'" "$(git rev-parse --show-toplevel)"
+	'
+
+	test_expect_success "$1: prefix" '
+		test_cmp_val "'"$4"'" "$(git rev-parse --show-prefix)"
+	'
+}
+
+test_foobar_root() {
+	test_expect_success 'add relative' '
+		test -z "$(cd / && git ls-files)" &&
+		git add foo/foome &&
+		git add foo/bar/barme &&
+		git add me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+	test_expect_success 'add absolute' '
+		test -z "$(cd / && git ls-files)" &&
+		git add /foo/foome &&
+		git add /foo/bar/barme &&
+		git add /me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+}
+
+test_foobar_foo() {
+	test_expect_success 'add relative' '
+		test -z "$(cd / && git ls-files)" &&
+		git add foome &&
+		git add bar/barme &&
+		git add ../me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+	test_expect_success 'add absolute' '
+		test -z "$(cd / && git ls-files)" &&
+		git add /foo/foome &&
+		git add /foo/bar/barme &&
+		git add /me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+}
+
+test_foobar_foobar() {
+	test_expect_success 'add relative' '
+		test -z "$(cd / && git ls-files)" &&
+		git add ../foome &&
+		git add barme &&
+		git add ../../me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+	test_expect_success 'add absolute' '
+		test -z "$(cd / && git ls-files)" &&
+		git add /foo/foome &&
+		git add /foo/bar/barme &&
+		git add /me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+}
+
+if ! test_have_prereq POSIXPERM || ! [ -w / ]; then
+	say "Dangerous test skipped. Read this test if you want to execute it"
+	test_done
+fi
+
+if [ "$IKNOWWHATIAMDOING" != "YES" ]; then
+	say "You must set env var IKNOWWHATIAMDOING=YES in order to run this test"
+	test_done
+fi
+
+ONE_SHA1=d00491fd7e5bb6fa28c517a0bb32b8b506539d4d
+
+test_expect_success 'setup' '
+	rm -rf /foo
+	mkdir /foo &&
+	mkdir /foo/bar &&
+	echo 1 > /foo/foome &&
+	echo 1 > /foo/bar/barme &&
+	echo 1 > /me
+'
+
+say "GIT_DIR absolute, GIT_WORK_TREE set"
+
+test_expect_success 'go to /' 'cd /'
+
+cat >ls.expected <<EOF
+100644 $ONE_SHA1 0	foo/bar/barme
+100644 $ONE_SHA1 0	foo/foome
+100644 $ONE_SHA1 0	me
+EOF
+
+export GIT_DIR="$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'abs gitdir, root' "$GIT_DIR" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /foo' 'cd /foo'
+
+test_vars 'abs gitdir, foo' "$GIT_DIR" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+
+test_vars 'abs gitdir, foo/bar' "$GIT_DIR" "/" "foo/bar/"
+test_foobar_foobar
+
+say "GIT_DIR relative, GIT_WORK_TREE set"
+
+test_expect_success 'go to /' 'cd /'
+
+export GIT_DIR="$(echo $TRASH_DIRECTORY|sed 's,^/,,')/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'rel gitdir, root' "$GIT_DIR" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /foo' 'cd /foo'
+
+export GIT_DIR="../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'rel gitdir, foo' "$TRASH_DIRECTORY/.git" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+
+export GIT_DIR="../../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'rel gitdir, foo/bar' "$TRASH_DIRECTORY/.git" "/" "foo/bar/"
+test_foobar_foobar
+
+say "GIT_DIR relative, GIT_WORK_TREE relative"
+
+test_expect_success 'go to /' 'cd /'
+
+export GIT_DIR="$(echo $TRASH_DIRECTORY|sed 's,^/,,')/.git"
+export GIT_WORK_TREE=.
+
+test_vars 'rel gitdir, root' "$GIT_DIR" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /' 'cd /foo'
+
+export GIT_DIR="../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=..
+
+test_vars 'rel gitdir, foo' "$TRASH_DIRECTORY/.git" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+
+export GIT_DIR="../../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=../..
+
+test_vars 'rel gitdir, foo/bar' "$TRASH_DIRECTORY/.git" "/" "foo/bar/"
+test_foobar_foobar
+
+say ".git at root"
+
+unset GIT_DIR
+unset GIT_WORK_TREE
+
+test_expect_success 'go to /' 'cd /'
+test_expect_success 'setup' '
+	rm -rf /.git
+	echo "Initialized empty Git repository in /.git/" > expected &&
+	git init > result &&
+	test_cmp expected result
+'
+
+test_vars 'auto gitdir, root' ".git" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /foo' 'cd /foo'
+test_vars 'auto gitdir, foo' "/.git" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+test_vars 'auto gitdir, foo/bar' "/.git" "/" "foo/bar/"
+test_foobar_foobar
+
+test_expect_success 'cleanup' 'rm -rf /.git'
+
+say "auto bare gitdir"
+
+# DESTROYYYYY!!!!!
+test_expect_failure 'setup' '
+	rm -rf /refs /objects /info /hooks
+	rm /*
+	cd / &&
+	echo "Initialized empty Git repository in /" > expected &&
+	git init --bare > result &&
+	test_cmp expected result
+'
+
+test_vars 'auto gitdir, root' "." "" ""
+
+test_expect_success 'go to /foo' 'cd /foo'
+
+test_vars 'auto gitdir, root' "/" "" ""
+
+test_done
diff --git a/t/t1509/excludes b/t/t1509/excludes
new file mode 100644
index 0000000..d4d21d3
--- /dev/null
+++ b/t/t1509/excludes
@@ -0,0 +1,14 @@
+*.o
+*~
+*.bak
+*.c
+*.h
+.git
+contrib
+Documentation
+git-gui
+gitk-git
+gitweb
+t/t4013
+t/t5100
+t/t5515
diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
new file mode 100755
index 0000000..407f2d2
--- /dev/null
+++ b/t/t1509/prepare-chroot.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+die() {
+	echo >&2 "$@"
+	exit 1
+}
+
+xmkdir() {
+	while [ -n "$1" ]; do
+		[ -d "$1" ] || mkdir "$1" || die "Unable to mkdir $1"
+		shift
+	done
+}
+
+R="$1"
+
+[ -n "$R" ] || die "Usage: prepare-chroot.sh <root>"
+[ -x git ] || die "This script needs to be executed at git source code's top directory"
+[ -x /bin/busybox ] || die "You need busybox"
+
+xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
+[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null"
+echo "root:x:0:0:root:/:/bin/sh" > "$R/etc/passwd"
+echo "$(id -nu):x:$(id -u):$(id -g)::$(pwd)/t:/bin/sh" >> "$R/etc/passwd"
+echo "root::0:root" > "$R/etc/group"
+echo "$(id -ng)::$(id -g):$(id -nu)" >> "$R/etc/group"
+
+[ -x "$R/bin/busybox" ] || cp /bin/busybox "$R/bin/busybox"
+[ -x "$R/bin/sh" ] || ln -s /bin/busybox "$R/bin/sh"
+[ -x "$R/bin/su" ] || ln -s /bin/busybox "$R/bin/su"
+
+mkdir -p "$R$(pwd)"
+rsync --exclude-from t/t1509/excludes -Ha . "$R$(pwd)"
+ldd git | grep '=> /' | sed 's,.* => *\([^ ]*\) .*,\1,' | while read i; do
+	mkdir -p "$R$(dirname $i)"
+	cp "$i" "$R/$i"
+done
+echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
-- 
1.7.0.rc2.182.g3adef
João Carlos Mendes Luís· Feb 10, 2010, 13:10 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 4/4] Add test for using Git at root of file system

Nguyễn Thái Ngọc Duy wrote:
> This kind of test requires a throw-away root filesystem so that it can
> play on. If you have such a system, go ahead, "chmod 777 /" and run
> this test manually ("make test" with root permission won't work).
>   

I've seen you have a prepare-chroot.sh file in there. Is it working or not? I mean, did you create a chrooted environment to test, or there was any problem with that?

Nguyen Thai Ngoc Duy· Feb 11, 2010, 10:01 UTC · re: João Carlos Mendes Luís · lore

Re: [PATCH 4/4] Add test for using Git at root of file system

2010/2/10 João Carlos Mendes Luís <jonny@jonny.eng.br>:
Show 12 quoted lines
>
>
> Nguyễn Thái Ngọc Duy wrote:
>>
>> This kind of test requires a throw-away root filesystem so that it can
>> play on. If you have such a system, go ahead, "chmod 777 /" and run
>> this test manually ("make test" with root permission won't work).
>>
>
> I've seen you have a prepare-chroot.sh file in there.  Is it working or not?
>  I mean, did you create a chrooted environment to test, or there was any
> problem with that?

That means if you already have a chroot environment, use it. Or you can use prepare-chroot.sh to create a new chroot environment. Yes I used prepare-chroot.sh for my testing.

-- 
Duy
Johannes Sixt· Feb 9, 2010, 19:10 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 1/4] make_absolute_path(): Do not append redundant slash

On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
Show 8 quoted lines
> @@ -54,8 +54,9 @@ const char *make_absolute_path(const char *path)
>  			if (len + strlen(last_elem) + 2 > PATH_MAX)
>  				die ("Too long path name: '%s/%s'",
>  						buf, last_elem);
> -			buf[len] = '/';
> -			strcpy(buf + len + 1, last_elem);
> +			if (*buf != '/' || buf[1] != '\0')
> +				buf[len++] = '/';

Huh? You are adding a slash unless buf is exactly "/". That is, when buf is "/foo/" you still add a slash? That's not exactly avoiding redundancy. (Disclaimer: I didn't analyze the rest of the function whether my claim is true.)

> +			strcpy(buf + len, last_elem);
>  			free(last_elem);
>  			last_elem = NULL;
>  		}
-- Hannes
Nguyen Thai Ngoc Duy· Feb 11, 2010, 10:00 UTC · re: Johannes Sixt · lore

Re: [PATCH 1/4] make_absolute_path(): Do not append redundant slash

On Wed, Feb 10, 2010 at 2:10 AM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 14 quoted lines
> On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>> @@ -54,8 +54,9 @@ const char *make_absolute_path(const char *path)
>>                       if (len + strlen(last_elem) + 2 > PATH_MAX)
>>                               die ("Too long path name: '%s/%s'",
>>                                               buf, last_elem);
>> -                     buf[len] = '/';
>> -                     strcpy(buf + len + 1, last_elem);
>> +                     if (*buf != '/' || buf[1] != '\0')
>> +                             buf[len++] = '/';
>
> Huh? You are adding a slash unless buf is exactly "/". That is, when buf
> is "/foo/" you still add a slash? That's not exactly avoiding redundancy.
> (Disclaimer: I didn't analyze the rest of the function whether my claim is
> true.)
buf is set by getcwd() so it should never be "/foo/" but doing
if (len && buf[len-1] != '/') buf[len++] = '/';
is probably clearer (and works on Windows too).
-- 
Duy

← back to recent threads