Richard W.M. Jones
2015-Jun-23 13:34 UTC
[Libguestfs] [PATCH] daemon: Rewrite prog_exists so it uses the actual PATH, not hard-coded list.
--- daemon/guestfsd.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 21b3600..c9cc8c5 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -243,9 +243,6 @@ 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", "/sbin:/usr/sbin:/bin:/usr/bin", 1); setenv ("SHELL", "/bin/sh", 1); @@ -1440,22 +1437,34 @@ mountable_to_string (const mountable_t *mountable) } } -/* Check program exists and is executable on $PATH. Actually, we - * just assume PATH contains the default entries (see main() above). - */ +/* Check program exists and is executable on $PATH. */ int prog_exists (const char *prog) { - 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); - if (access (buf, X_OK) == 0) + const char *pathc = getenv ("PATH"); + + if (!pathc) + return 0; + + const char *elem; + char *saveptr; + size_t len = strlen (pathc) + 1; + char path[len]; + strcpy (path, pathc); + + elem = strtok_r (path, ":", &saveptr); + while (elem) { + size_t n = strlen (elem) + strlen (prog) + 2; + char testprog[n]; + + snprintf (testprog, n, "%s/%s", elem, prog); + if (access (testprog, X_OK) == 0) return 1; + + elem = strtok_r (NULL, ":", &saveptr); } + + /* Not found. */ return 0; } -- 2.3.1
Pino Toscano
2015-Jun-23 17:58 UTC
Re: [Libguestfs] [PATCH] daemon: Rewrite prog_exists so it uses the actual PATH, not hard-coded list.
In data martedì 23 giugno 2015 14:34:18, Richard W.M. Jones ha scritto:> --- > daemon/guestfsd.c | 37 +++++++++++++++++++++++-------------- > 1 file changed, 23 insertions(+), 14 deletions(-) > > diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c > index 21b3600..c9cc8c5 100644 > --- a/daemon/guestfsd.c > +++ b/daemon/guestfsd.c > @@ -243,9 +243,6 @@ 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", "/sbin:/usr/sbin:/bin:/usr/bin", 1); > setenv ("SHELL", "/bin/sh", 1); > @@ -1440,22 +1437,34 @@ mountable_to_string (const mountable_t *mountable) > } > } > > -/* Check program exists and is executable on $PATH. Actually, we > - * just assume PATH contains the default entries (see main() above). > - */ > +/* Check program exists and is executable on $PATH. */ > int > prog_exists (const char *prog) > { > - 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); > - if (access (buf, X_OK) == 0) > + const char *pathc = getenv ("PATH"); > + > + if (!pathc) > + return 0; > + > + const char *elem; > + char *saveptr; > + size_t len = strlen (pathc) + 1; > + char path[len]; > + strcpy (path, pathc); > + > + elem = strtok_r (path, ":", &saveptr); > + while (elem) { > + size_t n = strlen (elem) + strlen (prog) + 2; > + char testprog[n]; > + > + snprintf (testprog, n, "%s/%s", elem, prog); > + if (access (testprog, X_OK) == 0) > return 1; > + > + elem = strtok_r (NULL, ":", &saveptr); > } > + > + /* Not found. */ > return 0; > }LGTM. The only minor note, "strlen (prog)" could be cached outside the while loop. -- Pino Toscano
Richard W.M. Jones
2015-Jun-23 20:24 UTC
Re: [Libguestfs] [PATCH] daemon: Rewrite prog_exists so it uses the actual PATH, not hard-coded list.
On Tue, Jun 23, 2015 at 07:58:52PM +0200, Pino Toscano wrote:> In data martedì 23 giugno 2015 14:34:18, Richard W.M. Jones ha scritto: > > --- > > daemon/guestfsd.c | 37 +++++++++++++++++++++++-------------- > > 1 file changed, 23 insertions(+), 14 deletions(-) > > > > diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c > > index 21b3600..c9cc8c5 100644 > > --- a/daemon/guestfsd.c > > +++ b/daemon/guestfsd.c > > @@ -243,9 +243,6 @@ 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", "/sbin:/usr/sbin:/bin:/usr/bin", 1); > > setenv ("SHELL", "/bin/sh", 1); > > @@ -1440,22 +1437,34 @@ mountable_to_string (const mountable_t *mountable) > > } > > } > > > > -/* Check program exists and is executable on $PATH. Actually, we > > - * just assume PATH contains the default entries (see main() above). > > - */ > > +/* Check program exists and is executable on $PATH. */ > > int > > prog_exists (const char *prog) > > { > > - 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); > > - if (access (buf, X_OK) == 0) > > + const char *pathc = getenv ("PATH"); > > + > > + if (!pathc) > > + return 0; > > + > > + const char *elem; > > + char *saveptr; > > + size_t len = strlen (pathc) + 1; > > + char path[len]; > > + strcpy (path, pathc); > > + > > + elem = strtok_r (path, ":", &saveptr); > > + while (elem) { > > + size_t n = strlen (elem) + strlen (prog) + 2; > > + char testprog[n]; > > + > > + snprintf (testprog, n, "%s/%s", elem, prog); > > + if (access (testprog, X_OK) == 0) > > return 1; > > + > > + elem = strtok_r (NULL, ":", &saveptr); > > } > > + > > + /* Not found. */ > > return 0; > > } > > LGTM. > > The only minor note, "strlen (prog)" could be cached outside the while > loop.Yup. Kind of hoping that gcc would do that, but I just checked the assembler output at -O2 and as best I can tell it doesn't :-( I will modify the source .. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Reasonably Related Threads
- Re: [PATCH] daemon: Rewrite prog_exists so it uses the actual PATH, not hard-coded list.
- [PATCH 0/3] library: improve handling of external tools
- [PATCH v2 0/3] library: improve handling of external tools
- [PATCH 0/6] tests: Fix handling of device API parameters (RHBZ#1477623).
- [PATCH 0/5] Use less stack.