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

Subsystems: the rest

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:25

"Alex Riesen" [off-list ref] writes:
quoted hunk
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?

-- >8 --
Fix work-tree related breakages

In set_work_tree(), variable rel needs to be reinitialized to
NULL on every call (it should not be static).

Make sure the incoming dir variable is not too long before
copying to the temporary buffer, and make sure chdir to the
resulting directory succeeds.

---

 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;

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:25

Hi,

On Thu, 2 Aug 2007, Junio C Hamano wrote:
Fix work-tree related breakages

In set_work_tree(), variable rel needs to be reinitialized to
NULL on every call (it should not be static).

Make sure the incoming dir variable is not too long before
copying to the temporary buffer, and make sure chdir to the
resulting directory succeeds.
ACK.

(Forget my patch, please)

Ciao,
Dscho

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

From: Alex Riesen <hidden>
Date: 2016-06-15 22:43:25

Junio C Hamano, Thu, Aug 02, 2007 23:58:41 +0200:
"Alex Riesen" [off-list ref] writes:
quoted
+		if (chdir(dir))
+			rel = NULL;
...
Shouldn't it die() instead, though?
Dunno. Don't like dying.
Consolidating two of your patches, would this be Ok?
Yes, but you may consider replacing strncpy with strlcpy:
+		memcpy(dir_buffer, dir, len - suffix_len);
+		dir_buffer[len - suffix_len] = '\0';
strlcpy(dir_buffer, dir, len - suffix_len + 1);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help