Re: [PATCH] Handle the errors from chdir in set_work_tree

From

Junio C Hamano <gitster@pobox.com>

Date

Aug 2, 2007, 21:58 UTC

Message-ID

<7v8x8ttx7y.fsf@assigned-by-dhcp.cox.net>

In-Reply-To

<81b0412b0708020827p174515b7tc05fefde77f7d7c4@mail.gmail.com>

"Alex Riesen" <raa.lkml@gmail.com> writes:

> These I haven't seen yet. Wouldn't like such a surprise though.
> ...
> @@ -220,8 +220,10 @@ const char *set_work_tree(const char *dir)
>  		if (!is_absolute_path(dir))
>  			set_git_dir(make_absolute_path(dir));
>  		dir = dir_buffer;
> -		chdir(dir);
> -		strcat(rel, "/");
> +		if (chdir(dir))
> +			rel = NULL;
> +		else
> +			strcat(rel, "/");
>  		inside_git_dir = 0;
>  	} else {
>  		rel = NULL;

Shouldn't it die() instead, though?

Consolidating two of your patches, would this be Ok?

---

 setup.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/setup.c b/setup.c
index 3653092..4945eb3 100644
--- a/setup.c
+++ b/setup.c
@@ -201,26 +201,32 @@ int is_inside_work_tree(void)
  */
 const char *set_work_tree(const char *dir)
 {
-	char dir_buffer[PATH_MAX];
-	static char buffer[PATH_MAX + 1], *rel = NULL;
-	int len, postfix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
+	char dir_buffer[PATH_MAX], *rel = NULL;
+	static char buffer[PATH_MAX + 1];
+	int len, suffix_len = strlen(DEFAULT_GIT_DIR_ENVIRONMENT) + 1;
 
 	/* strip the variable 'dir' of the postfix "/.git" if it has it */
 	len = strlen(dir);
-	if (len > postfix_len && !strcmp(dir + len - postfix_len,
-				"/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
-			strncpy(dir_buffer, dir, len - postfix_len);
+	if (len > suffix_len &&
+	    !strcmp(dir + len - suffix_len, "/" DEFAULT_GIT_DIR_ENVIRONMENT)) {
+		if ((len - suffix_len) >= sizeof(dir_buffer))
+			die("directory name too long");
+		memcpy(dir_buffer, dir, len - suffix_len);
+		dir_buffer[len - suffix_len] = '\0';
 
 		/* are we inside the default work tree? */
 		rel = get_relative_cwd(buffer, sizeof(buffer), dir_buffer);
 	}
+
 	/* if rel is set, the cwd is _not_ the current working tree */
 	if (rel && *rel) {
 		if (!is_absolute_path(dir))
 			set_git_dir(make_absolute_path(dir));
 		dir = dir_buffer;
-		chdir(dir);
-		strcat(rel, "/");
+		if (chdir(dir))
+			die("cannot chdir to %s: %s", dir, strerror(errno));
+		else
+			strcat(rel, "/");
 		inside_git_dir = 0;
 	} else {
 		rel = NULL;