PATCH: replacing exec*p with BB_EXEC*P

[Gabriel L. Somlo]

Denis, Bernhard,

Here's the patch that adds bb_execvp(). I compiled it
with static and dynamic linking (against glibc, I know static there is
buggy, but I just wanted to see how the binary size was affected :) ).
Here's the results (I'm building all but maybe two of the applets that
call execvp()):

arch.   linking         standalone      macro calls     bytes   
                        macro           bb_execvp()     saved


        static          1257820         1257820         0 
i386
        dynamic          512136          512136         0 


        static          1677276         1676860         416 
ppc
        dynamic          659600          659600         0 

No clue how much (if anything) we'd save on other architectures. Also,
not clear why dynamically linked binaries aren't affected (maybe
there's some padding/alignment thing, but I'm not an expert on the ELF
format :)

Also, if you *want* a bb_execlp(), we could do the following,
(shamelessly ripped from glibc):

int bb_execlp (const char *file, const char *arg, ...)
{
#define INITIAL_ARGV_MAX 1024
  size_t argv_max = INITIAL_ARGV_MAX;
  const char *initial_argv[INITIAL_ARGV_MAX];
  const char **argv = initial_argv;
  va_list args;

  argv[0] = arg;

  va_start (args, arg);
  unsigned int i = 0;
  while (argv[i++] != NULL)
    {
      if (i == argv_max)
        {
          argv_max *= 2;
          const char **nptr = xrealloc (argv == initial_argv ? NULL : argv,
                                       argv_max * sizeof (const char *));
          if (nptr == NULL)
            {
              if (argv != initial_argv)
                free (argv);
              return -1;
            }
          if (argv == initial_argv)
            /* We have to copy the already filled-in data ourselves.  */
            memcpy (nptr, argv, i * sizeof (const char *));

          argv = nptr;
        }

      argv[i] = va_arg (args, const char *);
    }
  va_end (args);

  int ret = bb_execvp (file, (char *const *) argv);
  if (argv != initial_argv)
    free (argv);

  return ret;
}

But given how few the places it's being called from, we'd end up maybe
increasing the size slightly instead of saving a few bytes.

(now, I don't know how many places call execl() with hardcoded paths
that should really be calling BB_EXECLP(), but that's something I'll
be looking into next :) )

OK, so given the meager space savings, do you guys still want this, or
should we stick with standalone macros ?

Here goes the patch.
Cheers,
--Gabriel

diff -NarU5 busybox-svn-17770.orig/libbb/execable.c busybox-svn-17770/libbb/execable.c
--- busybox-svn-17770.orig/libbb/execable.c	2007-02-04 16:25:51.000000000 -0500
+++ busybox-svn-17770/libbb/execable.c	2007-02-04 17:43:13.000000000 -0500
@@ -57,5 +57,15 @@
 		free(ret);
 		return 1;
 	}
 	return 0;
 }
+
+#ifdef ENABLE_FEATURE_EXEC_PREFER_APPLETS
+/* just like the real execvp, but try to launch an applet named 'file' first
+ */
+int bb_execvp(const char *file, char *const argv[])
+{
+	return execvp(find_applet_by_name(file) ? CONFIG_BUSYBOX_EXEC_PATH : file,
+					argv);
+}
+#endif
diff -NarU5 busybox-svn-17770.orig/include/libbb.h busybox-svn-17770/include/libbb.h
--- busybox-svn-17770.orig/include/libbb.h	2007-02-04 16:25:53.000000000 -0500
+++ busybox-svn-17770/include/libbb.h	2007-02-04 17:42:54.000000000 -0500
@@ -560,12 +560,12 @@
 int execable_file(const char *name);
 char *find_execable(const char *filename);
 int exists_execable(const char *filename);
 
 #ifdef ENABLE_FEATURE_EXEC_PREFER_APPLETS
-#define BB_EXECVP(prog,cmd) \
-	execvp((find_applet_by_name(prog)) ? CONFIG_BUSYBOX_EXEC_PATH : prog, cmd)
+int bb_execvp(const char *file, char *const argv[]);
+#define BB_EXECVP(prog,cmd) bb_execvp(prog,cmd)
 #define BB_EXECLP(prog,cmd,...) \
 	execlp((find_applet_by_name(prog)) ? CONFIG_BUSYBOX_EXEC_PATH : prog, cmd, __VA_ARGS__)
 #else
 #define BB_EXECVP(prog,cmd) execvp(prog,cmd)
 #define BB_EXECLP(prog,cmd,...) execlp(prog,cmd, __VA_ARGS__)