Re: [PATCH 2/2] git: continue alias lookup on EACCES errors

Jeff King [off-list ref] writes:

+static int file_in_path_is_nonexecutable(const char *file)
+{
+	const char *p = getenv("PATH");
+
+	if (!p)
+		return 0;
+
+	while (1) {
+		const char *end = strchrnul(p, ':');
+		const char *path;
+		struct stat st;
+
+		path = mkpath("%.*s/%s", (int)(end - p), p, file);

Given PATH=":/usr/bin:/bin" and file "frotz" (please call it "cmd" or
something, by the way), end points at the first colon and path becomes
"/frotz".  Oops?

+		if (!stat(path, &st) && access(path, X_OK) < 0)
+			return 1;
+
+		if (!*end)
+			break;
+
+		p = end + 1;
+	}
+
+	return 0;
+}
+
+int sane_execvp(const char *file, char * const argv[])
+{
+	int ret = execvp(file, argv);
+	if (ret < 0 && errno == EACCES && !file_in_path_is_nonexecutable(file))
+		errno = ENOENT;
+	return ret;

Double negation makes my head hurt, but unfortunately, we cannot rename it
to "executable_exists_on_path()" and negate its return value.

Anyway, the logic is to set errno to ENOENT when

 - We tried to exec, and got EACCES; and
 - There is a file on the PATH that lacks executable bit.

In such a case, the error from execvp() is not about the file it tried to
execute lacked executable bit, but there was nothing that match the name,
but it couldn't be certain because some directories were not readable.

OK.  I think I can follow that logic.

If there are more than one entry on PATH, and a system call made during
first round of the loop fails but a later round finds a non-executable
file, i.e.

	$ PATH=/nosuch:/home/peff/bin; export PATH
        $ >/home/peff/bin/frotz; chmod -x /home/peff/bin/frotz
        git frotz

we would get EACCES from execvp(), the first round runs stat("/nosuch/frotz")
and sets errno to ENOTDIR, and the second round runs stat() and access()
on "/home/peff/bin/frotz" and returns 1 to say "Yeah, there is a plain
file frotz that cannot be executed".

And sane_execvp() will return ENOTDIR?

So sane_execvp() would probably need to do a bit more (but not that much).

	if (ret < 0 && errno == EACCES)
		errno = file_in_path_is_nonexecutable(file) ? EACCES : ENOENT;
	return ret;

or something.