[Libguestfs] [PATCH] appliance: Set $PATH instead of hard-coding paths to binaries everywhere.

[Jim Meyering]

Richard W.M. Jones wrote:
> Subject: [PATCH] appliance: Set $PATH instead of hard-coding paths to binaries everywhere.
>
> Change the appliance so PATH includes common directories.  Thus
> we don't need to hard-code paths to binaries (eg. "/sbin/fdisk")
> everywhere.
...
> diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
> index 5265ab5..84b62ab 100644
> --- a/daemon/guestfsd.c
> +++ b/daemon/guestfsd.c
> @@ -220,8 +220,11 @@ main (int argc, char *argv[])
>    /* Set up a basic environment.  After we are called by /init the
>     * environment is essentially empty.
>     * https://bugzilla.redhat.com/show_bug.cgi?id=502074#c5
> +   *
> +   * NOTE: if you change $PATH, you must also change 'prog_exists'
> +   * function below.
>     */
> -  setenv ("PATH", "/usr/bin:/bin", 1);
> +  setenv ("PATH", "/sbin:/usr/sbin:/bin:/usr/bin", 1);
>    setenv ("SHELL", "/bin/sh", 1);
>    setenv ("LC_ALL", "C", 1);
>
> @@ -1056,6 +1059,25 @@ device_name_translation (char *device, const char *func)
>    goto error;
>  }
>
> +/* Check program exists and is executable on $PATH.  Actually, we
> + * just assume PATH contains the default entries (see main() above).
> + */
> +int
> +prog_exists (const char *prog)
> +{
> +  static const char *dirs[] =

Hi Rich,

Nice improvement.

You can add a "const" here.

  static const char *const dirs[] =

> +    { "/sbin", "/usr/sbin", "/bin", "/usr/bin" };
> +  size_t i;
> +  char buf[1024];
> +
> +  for (i = 0; i < sizeof dirs / sizeof dirs[0]; ++i) {
> +    snprintf (buf, sizeof buf, "%s/%s", dirs[i], prog);

You might want to check for failure here.
This can misbehave in a very twisted manner if there is
ever a "prog" value long enough that snprintf(...) >= sizeof buf,
and (this is the twisted part) the 1023-byte prefix *does*
happen to name an existing executable, the caller would end up
invoking the wrong executable.

> +    if (access (buf, X_OK) == 0)
> +      return 1;
> +  }
> +  return 0;
> +}

Other than that, it all looks fine.