which: fix TODO with NOFORK+malloc_failure misbehaving

function                                             old     new   delta
find_executable                                       86     104     +18
which_main                                           202     194      -8
executable_exists                                     66      51     -15
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/2 up/down: 18/-23)             Total: -5 bytes

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

1 files changed, 8 insertions, 8 deletions

diff --git a/debianutils/which.c b/debianutils/which.c
index 3bd54ac42..02f77a216 100644
--- a/debianutils/which.c
+++ b/debianutils/which.c
@@ -30,12 +30,15 @@
 int which_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int which_main(int argc UNUSED_PARAM, char **argv)
 {
-        const char *env_path;
+        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 = bb_default_root_path;
+                /* env_path must be writable, and must not alloc, so... */
+                env_path = strcpy(buf, bb_default_root_path);
 
         getopt32(argv, "^" "a" "\0" "-1"/*at least one arg*/);
         argv += optind;
@@ -51,20 +54,17 @@ int which_main(int argc UNUSED_PARAM, char **argv)
                         }
                 } else {
                         char *path;
-                        char *tmp;
                         char *p;
 
-                        path = tmp = xstrdup(env_path);
-//NOFORK FIXME: nested xmallocs (one is inside find_executable())
-//can leak memory on failure
-                        while ((p = find_executable(*argv, &tmp)) != NULL) {
+                        path = env_path;
+                        /* NOFORK NB: xmalloc inside find_executable(), must have no allocs above! */
+                        while ((p = find_executable(*argv, &path)) != NULL) {
                                 missing = 0;
                                 puts(p);
                                 free(p);
                                 if (!option_mask32) /* -a not set */
                                         break;
                         }
-                        free(path);
                 }
                 status |= missing;
         } while (*++argv);