libbb: modify find_executable() to not temporarily write to PATH

This allows to simplify "which" applet code

function                                             old     new   delta
find_executable                                       93     111     +18
which_main                                           191     177     -14
builtin_source                                       316     294     -22
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/2 up/down: 18/-36)            Total: -18 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

debianutils/which.c

@@ -31,15 +31,12 @@
 int which_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int which_main(int argc UNUSED_PARAM, char **argv)
 {
-	char *env_path;
+	const char *env_path;
 	int status = 0;
-	/* This sizeof(): bb_default_root_path is shorter than BB_PATH_ROOT_PATH */
-	char buf[sizeof(BB_PATH_ROOT_PATH)];
 
 	env_path = getenv("PATH");
 	if (!env_path)
-		/* env_path must be writable, and must not alloc, so... */
+		env_path = bb_default_root_path;
-		env_path = strcpy(buf, bb_default_root_path);
 
 	getopt32(argv, "^" "a" "\0" "-1"/*at least one arg*/);
 	argv += optind;
@@ -54,7 +51,7 @@ int which_main(int argc UNUSED_PARAM, char **argv)
 				puts(*argv);
 			}
 		} else {
-			char *path;
+			const char *path;
 			char *p;
 
 			path = env_path;

include/libbb.h

@@ -1227,7 +1227,7 @@ void FAST_FUNC update_utmp_DEAD_PROCESS(pid_t pid);
 
 
 int file_is_executable(const char *name) FAST_FUNC;
-char *find_executable(const char *filename, char **PATHp) FAST_FUNC;
+char *find_executable(const char *filename, const char **PATHp) FAST_FUNC;
 int executable_exists(const char *filename) FAST_FUNC;
 
 /* BB_EXECxx always execs (it's not doing NOFORK/NOEXEC stuff),

libbb/executable.c

@@ -21,14 +21,11 @@ int FAST_FUNC file_is_executable(const char *name)
 /* search (*PATHp) for an executable file;
  * return allocated string containing full path if found;
  *  PATHp points to the component after the one where it was found
- *  (or NULL),
+ *  (or NULL if found in last component),
  *  you may call find_executable again with this PATHp to continue
- *  (if it's not NULL).
+ * return NULL otherwise (PATHp is undefined)
- * return NULL otherwise; (PATHp is undefined)
- * in all cases (*PATHp) contents are temporarily modified
- * but are restored on return (s/:/NUL/ and back).
  */
-char* FAST_FUNC find_executable(const char *filename, char **PATHp)
+char* FAST_FUNC find_executable(const char *name, const char **PATHp)
 {
 	/* About empty components in $PATH:
 	 * http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
@@ -38,38 +35,45 @@ char* FAST_FUNC find_executable(const char *filename, char **PATHp)
 	 * initial colon preceding the rest of the list, or as a trailing colon
 	 * following the rest of the list.
 	 */
-	char *p, *n;
+	char *p = (char*) *PATHp;
 
-	p = *PATHp;
+	if (!p)
-	while (p) {
+		return NULL;
-		int ex;
+	while (1) {
+		const char *end = strchrnul(p, ':');
+		int sz = end - p;
 
-		n = strchr(p, ':');
+		if (sz != 0) {
-		if (n) *n = '\0';
+			p = xasprintf("%.*s/%s", sz, p, name);
-		p = concat_path_file(
+		} else {
-			p[0] ? p : ".", /* handle "::" case */
+			/* We have xxx::yyy in $PATH,
-			filename
+			 * it means "use current dir" */
-		);
+			p = xstrdup(name);
-		ex = file_is_executable(p);
+// A bit of discrepancy wrt the path used if file is found here.
-		if (n) *n++ = ':';
+// bash 5.2.15 "type" returns "./NAME".
-		if (ex) {
+// GNU which v2.21 returns "/CUR/DIR/NAME".
-			*PATHp = n;
+// With -a, both skip over all colons: xxx::::yyy is the same as xxx::yyy,
+// current dir is not tried the second time.
+		}
+		if (file_is_executable(p)) {
+			*PATHp = (*end ? end+1 : NULL);
 			return p;
 		}
 		free(p);
-		p = n;
+		if (*end == '\0')
-	} /* on loop exit p == NULL */
+			return NULL;
-	return p;
+		p = (char *) end + 1;
+	}
 }
 
 /* search $PATH for an executable file;
  * return 1 if found;
  * return 0 otherwise;
  */
-int FAST_FUNC executable_exists(const char *filename)
+int FAST_FUNC executable_exists(const char *name)
 {
-	char *path = getenv("PATH");
+	const char *path = getenv("PATH");
-	char *ret = find_executable(filename, &path);
+	char *ret = find_executable(name, &path);
 	free(ret);
 	return ret != NULL;
 }

shell/hush.c

@@ -8207,41 +8207,34 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp)
 //TODO: shares code with find_executable() in libbb, factor out?
 static char *find_in_PATH(const char *name)
 {
-	char *ret;
+	char *p = (char*) get_local_var_value("PATH");
-	const char *PATH = get_local_var_value("PATH");
 
-	if (!PATH)
+	if (!p)
 		return NULL;
-	ret = NULL;
 	while (1) {
-		const char *end = strchrnul(PATH, ':');
+		const char *end = strchrnul(p, ':');
-		int sz = end - PATH; /* must be int! */
+		int sz = end - p; /* must be int! */
 
-		free(ret);
 		if (sz != 0) {
-			ret = xasprintf("%.*s/%s", sz, PATH, name);
+			p = xasprintf("%.*s/%s", sz, p, name);
 		} else {
-			/* We have xxx::yyyy in $PATH,
+			/* We have xxx::yyy in $PATH,
 			 * it means "use current dir" */
-			ret = xstrdup(name);
+			p = xstrdup(name);
 		}
-		if (access(ret, F_OK) == 0)
+		if (access(p, F_OK) == 0)
-			break;
+			return p;
-
+		free(p);
-		if (*end == '\0') {
+		if (*end == '\0')
-			free(ret);
 			return NULL;
-		}
+		p = (char *) end + 1;
-		PATH = end + 1;
 	}
-
-	return ret;
 }
 
 #if ENABLE_HUSH_TYPE || ENABLE_HUSH_COMMAND
 static char *find_executable_in_PATH(const char *name)
 {
-	char *PATH;
+	const char *PATH;
 	if (strchr(name, '/')) {
 		/* Name with '/' is tested verbatim, with no PATH traversal:
 		 * "cd /bin; type ./cat" should print "./cat is ./cat",
@@ -8251,7 +8244,7 @@ static char *find_executable_in_PATH(const char *name)
 			return xstrdup(name);
 		return NULL;
 	}
-	PATH = (char*)get_local_var_value("PATH");
+	PATH = get_local_var_value("PATH");
 	return find_executable(name, &PATH); /* path == NULL is ok */
 }
 #endif