Re: [PATCH 2/7] builtin-help: change the current directory back in list_commands_in_dir()

Miklos Vajna [off-list ref] writes:

That function used to do a chdir() without switching back to the
original directory. That was not a problem till this function was used
only inside builtin-help, but once other builtins use it as well, this
is a problem, for example when the object database path is relative.

Why does it even need to chdir() around to begin with?  Doesn't opendir()
work just fine with relative path as an input?

Hi,

On Sun, 27 Jul 2008, Junio C Hamano wrote:

Miklos Vajna [off-list ref] writes:

quoted

That function used to do a chdir() without switching back to the 
original directory. That was not a problem till this function was used 
only inside builtin-help, but once other builtins use it as well, this 
is a problem, for example when the object database path is relative.

Why does it even need to chdir() around to begin with?  Doesn't 
opendir() work just fine with relative path as an input?

It is a consequence of list_commands_in_dir() trying to be cute, and not 
having to construct the full path for the is_executable() check.

Will post a fix in a few minutes.

Ciao,
Dscho

The function list_commands_in_dir() tried to be lazy and just chdir()
to the directory which entries it listed, so that the check if the
file is executable could be done on dir->d_name.

However, there is no good reason to jump around wildly just to find
all Git commands.

Instead, have a strbuf and construct the full path dynamically.

Signed-off-by: Johannes Schindelin <redacted>
---
 help.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/help.c b/help.c
index 480befe..7af6582 100644
--- a/help.c
+++ b/help.c

@@ -426,17 +426,24 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,
 	int prefix_len = strlen(prefix);
 	DIR *dir = opendir(path);
 	struct dirent *de;
+	struct strbuf buf = STRBUF_INIT;
+	int len;
 
-	if (!dir || chdir(path))
+	if (!dir)
 		return 0;
 
+	strbuf_addf(&buf, "%s/", path);
+	len = buf.len;
+
 	while ((de = readdir(dir)) != NULL) {
 		int entlen;
 
 		if (prefixcmp(de->d_name, prefix))
 			continue;
 
-		if (!is_executable(de->d_name))
+		strbuf_setlen(&buf, len);
+		strbuf_addstr(&buf, de->d_name);
+		if (!is_executable(buf.buf))
 			continue;
 
 		entlen = strlen(de->d_name) - prefix_len;

@@ -449,6 +456,7 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,
 		add_cmdname(cmds, de->d_name + prefix_len, entlen);
 	}
 	closedir(dir);
+	strbuf_release(&buf);
 
 	return longest;
 }

-- 
1.6.0.rc0.97.ge2309