Richard W.M. Jones
2010-Mar-26 11:30 UTC
[Libguestfs] [PATCH] appliance: Set $PATH instead of hard-coding paths to binaries everywhere.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -------------- next part -------------->From 0420307349a71b7b9af58003c2591fc33b7939af Mon Sep 17 00:00:00 2001From: Richard Jones <rjones at redhat.com> Date: Fri, 26 Mar 2010 10:10:16 +0000 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. --- appliance/init | 3 ++- daemon/blkid.c | 2 +- daemon/blockdev.c | 2 +- daemon/daemon.h | 2 ++ daemon/ext2.c | 30 +++++++++++++++--------------- daemon/fsck.c | 2 +- daemon/grub.c | 2 +- daemon/guestfsd.c | 28 +++++++++++++++++++++++++--- daemon/lvm.c | 41 ++++++++++++++++++++--------------------- daemon/mkfs.c | 2 +- daemon/modprobe.c | 5 ++--- daemon/parted.c | 6 +++--- daemon/scrub.c | 3 +-- daemon/sfdisk.c | 4 ++-- daemon/swap.c | 22 +++++++++++----------- daemon/zerofree.c | 5 ++--- src/generator.ml | 2 +- 17 files changed, 91 insertions(+), 70 deletions(-) diff --git a/appliance/init b/appliance/init index 84ee96e..fe01e00 100755 --- a/appliance/init +++ b/appliance/init @@ -2,7 +2,8 @@ echo Starting /init script ... -PATH=/sbin:/usr/sbin:$PATH +PATH=/sbin:/usr/sbin:/bin:/usr/bin +export PATH mount -t proc /proc /proc mount -t sysfs /sys /sys diff --git a/daemon/blkid.c b/daemon/blkid.c index 4c77b82..dcfac65 100644 --- a/daemon/blkid.c +++ b/daemon/blkid.c @@ -34,7 +34,7 @@ do_vfs_type (const char *device) int r; r = command (&out, &err, - "/sbin/blkid", "-o", "value", "-s", "TYPE", device, NULL); + "blkid", "-o", "value", "-s", "TYPE", device, NULL); if (r == -1) { reply_with_error ("%s: %s", device, err); free (out); diff --git a/daemon/blockdev.c b/daemon/blockdev.c index 95e5246..3df1807 100644 --- a/daemon/blockdev.c +++ b/daemon/blockdev.c @@ -38,7 +38,7 @@ call_blockdev (const char *device, const char *switc, int extraarg, int prints) int64_t rv; char *out, *err; const char *argv[] = { - "/sbin/blockdev", + "blockdev", switc, NULL, NULL, diff --git a/daemon/daemon.h b/daemon/daemon.h index 777cf33..19dd69c 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -70,6 +70,8 @@ extern void trim (char *str); extern int device_name_translation (char *device, const char *func); +extern int prog_exists (const char *prog); + extern void udev_settle (void); /* This just stops gcc from giving a warning about our custom diff --git a/daemon/ext2.c b/daemon/ext2.c index c90ee05..3758f4e 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -43,11 +43,11 @@ e2prog (char *name) p++; *p = '4'; - if (access (name, X_OK) == 0) + if (prog_exists (name)) return 0; *p = '2'; - if (access (name, X_OK) == 0) + if (prog_exists (name)) return 0; reply_with_error ("cannot find required program %s", name); @@ -63,7 +63,7 @@ do_tune2fs_l (const char *device) char **ret = NULL; int size = 0, alloc = 0; - char prog[] = "/sbin/tune2fs"; + char prog[] = "tune2fs"; if (e2prog (prog) == -1) return NULL; @@ -151,7 +151,7 @@ do_set_e2label (const char *device, const char *label) int r; char *err; - char prog[] = "/sbin/e2label"; + char prog[] = "e2label"; if (e2prog (prog) == -1) return -1; @@ -172,7 +172,7 @@ do_get_e2label (const char *device) int r, len; char *out, *err; - char prog[] = "/sbin/e2label"; + char prog[] = "e2label"; if (e2prog (prog) == -1) return NULL; @@ -200,7 +200,7 @@ do_set_e2uuid (const char *device, const char *uuid) int r; char *err; - char prog[] = "/sbin/tune2fs"; + char prog[] = "tune2fs"; if (e2prog (prog) == -1) return -1; @@ -225,7 +225,7 @@ do_get_e2uuid (const char *device) * to use tune2fs -l and then look for a particular string in * the output. */ - char prog[] = "/sbin/tune2fs"; + char prog[] = "tune2fs"; if (e2prog (prog) == -1) return NULL; @@ -285,7 +285,7 @@ do_resize2fs (const char *device) char *err; int r; - char prog[] = "/sbin/resize2fs"; + char prog[] = "resize2fs"; if (e2prog (prog) == -1) return -1; @@ -306,7 +306,7 @@ do_e2fsck_f (const char *device) char *err; int r; - char prog[] = "/sbin/e2fsck"; + char prog[] = "e2fsck"; if (e2prog (prog) == -1) return -1; @@ -334,7 +334,7 @@ do_mke2journal (int blocksize, const char *device) char *err; int r; - char prog[] = "/sbin/mke2fs"; + char prog[] = "mke2fs"; if (e2prog (prog) == -1) return -1; @@ -360,7 +360,7 @@ do_mke2journal_L (int blocksize, const char *label, const char *device) char *err; int r; - char prog[] = "/sbin/mke2fs"; + char prog[] = "mke2fs"; if (e2prog (prog) == -1) return -1; @@ -387,7 +387,7 @@ do_mke2journal_U (int blocksize, const char *uuid, const char *device) char *err; int r; - char prog[] = "/sbin/mke2fs"; + char prog[] = "mke2fs"; if (e2prog (prog) == -1) return -1; @@ -415,7 +415,7 @@ do_mke2fs_J (const char *fstype, int blocksize, const char *device, char *err; int r; - char prog[] = "/sbin/mke2fs"; + char prog[] = "mke2fs"; if (e2prog (prog) == -1) return -1; @@ -446,7 +446,7 @@ do_mke2fs_JL (const char *fstype, int blocksize, const char *device, char *err; int r; - char prog[] = "/sbin/mke2fs"; + char prog[] = "mke2fs"; if (e2prog (prog) == -1) return -1; @@ -477,7 +477,7 @@ do_mke2fs_JU (const char *fstype, int blocksize, const char *device, char *err; int r; - char prog[] = "/sbin/mke2fs"; + char prog[] = "mke2fs"; if (e2prog (prog) == -1) return -1; diff --git a/daemon/fsck.c b/daemon/fsck.c index 932f1c5..e452adc 100644 --- a/daemon/fsck.c +++ b/daemon/fsck.c @@ -32,7 +32,7 @@ do_fsck (const char *fstype, const char *device) char *err; int r; - r = commandr (NULL, &err, "/sbin/fsck", "-a", "-t", fstype, device, NULL); + r = commandr (NULL, &err, "fsck", "-a", "-t", fstype, device, NULL); if (r == -1) { reply_with_error ("%s: %s", device, err); free (err); diff --git a/daemon/grub.c b/daemon/grub.c index 579cc6b..f394eef 100644 --- a/daemon/grub.c +++ b/daemon/grub.c @@ -37,7 +37,7 @@ do_grub_install (const char *root, const char *device) return -1; } - r = command (NULL, &err, "/sbin/grub-install", buf, device, NULL); + r = command (NULL, &err, "grub-install", buf, device, NULL); free (buf); if (r == -1) { 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[] + { "/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) + return 1; + } + return 0; +} + /* LVM and other commands aren't synchronous, especially when udev is * involved. eg. You can create or remove some device, but the /dev * device node won't appear until some time later. This means that @@ -1073,6 +1095,6 @@ device_name_translation (char *device, const char *func) void udev_settle (void) { - (void) command (NULL, NULL, "/sbin/udevadm", "settle", NULL); - (void) command (NULL, NULL, "/sbin/udevsettle", NULL); + (void) command (NULL, NULL, "udevadm", "settle", NULL); + (void) command (NULL, NULL, "udevsettle", NULL); } diff --git a/daemon/lvm.c b/daemon/lvm.c index 82cdf3f..e4fa54e 100644 --- a/daemon/lvm.c +++ b/daemon/lvm.c @@ -31,8 +31,7 @@ int optgroup_lvm2_available (void) { - int r = access ("/sbin/lvm", X_OK); - return r == 0; + return prog_exists ("lvm"); } /* LVM actions. Keep an eye on liblvm, although at the time @@ -101,7 +100,7 @@ do_pvs (void) int r; r = command (&out, &err, - "/sbin/lvm", "pvs", "-o", "pv_name", "--noheadings", NULL); + "lvm", "pvs", "-o", "pv_name", "--noheadings", NULL); if (r == -1) { reply_with_error ("%s", err); free (out); @@ -121,7 +120,7 @@ do_vgs (void) int r; r = command (&out, &err, - "/sbin/lvm", "vgs", "-o", "vg_name", "--noheadings", NULL); + "lvm", "vgs", "-o", "vg_name", "--noheadings", NULL); if (r == -1) { reply_with_error ("%s", err); free (out); @@ -141,7 +140,7 @@ do_lvs (void) int r; r = command (&out, &err, - "/sbin/lvm", "lvs", + "lvm", "lvs", "-o", "vg_name,lv_name", "--noheadings", "--separator", "/", NULL); if (r == -1) { @@ -185,7 +184,7 @@ do_pvcreate (const char *device) int r; r = command (NULL, &err, - "/sbin/lvm", "pvcreate", device, NULL); + "lvm", "pvcreate", device, NULL); if (r == -1) { reply_with_error ("%s", err); free (err); @@ -212,7 +211,7 @@ do_vgcreate (const char *volgroup, char *const *physvols) reply_with_perror ("malloc"); return -1; } - argv[0] = "/sbin/lvm"; + argv[0] = "lvm"; argv[1] = "vgcreate"; argv[2] = volgroup; for (i = 3; i <= argc; ++i) @@ -242,7 +241,7 @@ do_lvcreate (const char *logvol, const char *volgroup, int mbytes) snprintf (size, sizeof size, "%d", mbytes); r = command (NULL, &err, - "/sbin/lvm", "lvcreate", + "lvm", "lvcreate", "-L", size, "-n", logvol, volgroup, NULL); if (r == -1) { reply_with_error ("%s", err); @@ -267,7 +266,7 @@ do_lvresize (const char *logvol, int mbytes) snprintf (size, sizeof size, "%d", mbytes); r = command (NULL, &err, - "/sbin/lvm", "lvresize", + "lvm", "lvresize", "-L", size, logvol, NULL); if (r == -1) { reply_with_error ("%s", err); @@ -295,7 +294,7 @@ do_lvm_remove_all (void) return -1; for (i = 0; xs[i] != NULL; ++i) { - r = command (NULL, &err, "/sbin/lvm", "lvremove", "-f", xs[i], NULL); + r = command (NULL, &err, "lvm", "lvremove", "-f", xs[i], NULL); if (r == -1) { reply_with_error ("lvremove: %s: %s", xs[i], err); free (err); @@ -312,7 +311,7 @@ do_lvm_remove_all (void) return -1; for (i = 0; xs[i] != NULL; ++i) { - r = command (NULL, &err, "/sbin/lvm", "vgremove", "-f", xs[i], NULL); + r = command (NULL, &err, "lvm", "vgremove", "-f", xs[i], NULL); if (r == -1) { reply_with_error ("vgremove: %s: %s", xs[i], err); free (err); @@ -329,7 +328,7 @@ do_lvm_remove_all (void) return -1; for (i = 0; xs[i] != NULL; ++i) { - r = command (NULL, &err, "/sbin/lvm", "pvremove", "-f", xs[i], NULL); + r = command (NULL, &err, "lvm", "pvremove", "-f", xs[i], NULL); if (r == -1) { reply_with_error ("pvremove: %s: %s", xs[i], err); free (err); @@ -353,7 +352,7 @@ do_lvremove (const char *device) int r; r = command (NULL, &err, - "/sbin/lvm", "lvremove", "-f", device, NULL); + "lvm", "lvremove", "-f", device, NULL); if (r == -1) { reply_with_error ("%s", err); free (err); @@ -374,7 +373,7 @@ do_vgremove (const char *device) int r; r = command (NULL, &err, - "/sbin/lvm", "vgremove", "-f", device, NULL); + "lvm", "vgremove", "-f", device, NULL); if (r == -1) { reply_with_error ("%s", err); free (err); @@ -395,7 +394,7 @@ do_pvremove (const char *device) int r; r = command (NULL, &err, - "/sbin/lvm", "pvremove", "-ff", device, NULL); + "lvm", "pvremove", "-ff", device, NULL); if (r == -1) { reply_with_error ("%s", err); free (err); @@ -416,7 +415,7 @@ do_pvresize (const char *device) int r; r = command (NULL, &err, - "/sbin/lvm", "pvresize", device, NULL); + "lvm", "pvresize", device, NULL); if (r == -1) { reply_with_error ("%s: %s", device, err); free (err); @@ -441,7 +440,7 @@ do_vg_activate (int activate, char *const *volgroups) return -1; } - argv[0] = "/sbin/lvm"; + argv[0] = "lvm"; argv[1] = "vgchange"; argv[2] = "-a"; argv[3] = activate ? "y" : "n"; @@ -476,7 +475,7 @@ do_lvrename (const char *logvol, const char *newlogvol) int r; r = command (NULL, &err, - "/sbin/lvm", "lvrename", + "lvm", "lvrename", logvol, newlogvol, NULL); if (r == -1) { reply_with_error ("%s -> %s: %s", logvol, newlogvol, err); @@ -498,7 +497,7 @@ do_vgrename (const char *volgroup, const char *newvolgroup) int r; r = command (NULL, &err, - "/sbin/lvm", "vgrename", + "lvm", "vgrename", volgroup, newvolgroup, NULL); if (r == -1) { reply_with_error ("%s -> %s: %s", volgroup, newvolgroup, err); @@ -519,7 +518,7 @@ get_lvm_field (const char *cmd, const char *field, const char *device) char *out; char *err; int r = command (&out, &err, - "/sbin/lvm", cmd, + "lvm", cmd, "--unbuffered", "--noheadings", "-o", field, device, NULL); if (r == -1) { @@ -559,7 +558,7 @@ get_lvm_fields (const char *cmd, const char *field, const char *device) char *out; char *err; int r = command (&out, &err, - "/sbin/lvm", cmd, + "lvm", cmd, "--unbuffered", "--noheadings", "-o", field, device, NULL); if (r == -1) { diff --git a/daemon/mkfs.c b/daemon/mkfs.c index 2beb82a..6735d24 100644 --- a/daemon/mkfs.c +++ b/daemon/mkfs.c @@ -40,7 +40,7 @@ mkfs (const char *fstype, const char *device, int r; char *err; - argv[i++] = "/sbin/mkfs"; + argv[i++] = "mkfs"; argv[i++] = "-t"; argv[i++] = fstype; diff --git a/daemon/modprobe.c b/daemon/modprobe.c index ac62b34..0d7318c 100644 --- a/daemon/modprobe.c +++ b/daemon/modprobe.c @@ -28,15 +28,14 @@ int optgroup_linuxmodules_available (void) { - int r = access ("/sbin/modprobe", X_OK); - return r == 0; + return prog_exists ("modprobe"); } int do_modprobe (const char *module) { char *err; - int r = command (NULL, &err, "/sbin/modprobe", module, NULL); + int r = command (NULL, &err, "modprobe", module, NULL); if (r == -1) { reply_with_error ("%s", err); diff --git a/daemon/parted.c b/daemon/parted.c index 3b656bc..99417c2 100644 --- a/daemon/parted.c +++ b/daemon/parted.c @@ -46,7 +46,7 @@ recover_blkrrpart (const char *device, const char *err) "Error informing the kernel about modifications to partition")) return -1; - r = command (NULL, NULL, "/sbin/blockdev", "--rereadpt", device, NULL); + r = command (NULL, NULL, "blockdev", "--rereadpt", device, NULL); if (r == -1) return -1; @@ -61,7 +61,7 @@ recover_blkrrpart (const char *device, const char *err) char *err; \ \ r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR, \ - "/sbin/parted", "-s", "--", (device), __VA_ARGS__); \ + "parted", "-s", "--", (device), __VA_ARGS__); \ if (r == -1) { \ if (recover_blkrrpart ((device), err) == -1) { \ reply_with_error ("%s: parted: %s: %s", __func__, (device), err); \ @@ -229,7 +229,7 @@ print_partition_table (const char *device) int r; char **lines; - r = command (&out, &err, "/sbin/parted", "-m", "--", device, + r = command (&out, &err, "parted", "-m", "--", device, "unit", "b", "print", NULL); if (r == -1) { diff --git a/daemon/scrub.c b/daemon/scrub.c index 08c6d47..573385f 100644 --- a/daemon/scrub.c +++ b/daemon/scrub.c @@ -31,8 +31,7 @@ int optgroup_scrub_available (void) { - int r = access ("/usr/bin/scrub", X_OK); - return r == 0; + return prog_exists ("scrub"); } int diff --git a/daemon/sfdisk.c b/daemon/sfdisk.c index 14297e7..20d7dc8 100644 --- a/daemon/sfdisk.c +++ b/daemon/sfdisk.c @@ -38,7 +38,7 @@ sfdisk (const char *device, int n, int cyls, int heads, int sectors, char buf[256]; int i; - strcpy (buf, "/sbin/sfdisk"); + strcpy (buf, "sfdisk"); if (n > 0) sprintf (buf + strlen (buf), " -N %d", n); @@ -122,7 +122,7 @@ sfdisk_flag (const char *device, const char *flag) char *out, *err; int r; - r = command (&out, &err, "/sbin/sfdisk", flag, device, NULL); + r = command (&out, &err, "sfdisk", flag, device, NULL); if (r == -1) { reply_with_error ("%s: %s", device, err); free (out); diff --git a/daemon/swap.c b/daemon/swap.c index fbeb895..67d553a 100644 --- a/daemon/swap.c +++ b/daemon/swap.c @@ -39,7 +39,7 @@ optgroup_linuxfsuuid_available (void) int av; /* Ignore return code - mkswap --help *will* fail. */ - command (NULL, &err, "/sbin/mkswap", "--help", NULL); + command (NULL, &err, "mkswap", "--help", NULL); av = strstr (err, "-U") != NULL; free (err); @@ -53,9 +53,9 @@ mkswap (const char *device, const char *flag, const char *value) int r; if (!flag) - r = command (NULL, &err, "/sbin/mkswap", "-f", device, NULL); + r = command (NULL, &err, "mkswap", "-f", device, NULL); else - r = command (NULL, &err, "/sbin/mkswap", "-f", flag, value, device, NULL); + r = command (NULL, &err, "mkswap", "-f", flag, value, device, NULL); if (r == -1) { reply_with_error ("%s", err); @@ -128,13 +128,13 @@ swaponoff (const char *cmd, const char *flag, const char *value) int do_swapon_device (const char *device) { - return swaponoff ("/sbin/swapon", NULL, device); + return swaponoff ("swapon", NULL, device); } int do_swapoff_device (const char *device) { - return swaponoff ("/sbin/swapoff", NULL, device); + return swaponoff ("swapoff", NULL, device); } int @@ -149,7 +149,7 @@ do_swapon_file (const char *path) return -1; } - r = swaponoff ("/sbin/swapon", NULL, buf); + r = swaponoff ("swapon", NULL, buf); free (buf); return r; } @@ -166,7 +166,7 @@ do_swapoff_file (const char *path) return -1; } - r = swaponoff ("/sbin/swapoff", NULL, buf); + r = swaponoff ("swapoff", NULL, buf); free (buf); return r; } @@ -174,23 +174,23 @@ do_swapoff_file (const char *path) int do_swapon_label (const char *label) { - return swaponoff ("/sbin/swapon", "-L", label); + return swaponoff ("swapon", "-L", label); } int do_swapoff_label (const char *label) { - return swaponoff ("/sbin/swapoff", "-L", label); + return swaponoff ("swapoff", "-L", label); } int do_swapon_uuid (const char *uuid) { - return swaponoff ("/sbin/swapon", "-U", uuid); + return swaponoff ("swapon", "-U", uuid); } int do_swapoff_uuid (const char *uuid) { - return swaponoff ("/sbin/swapoff", "-U", uuid); + return swaponoff ("swapoff", "-U", uuid); } diff --git a/daemon/zerofree.c b/daemon/zerofree.c index 84073dd..f98c69b 100644 --- a/daemon/zerofree.c +++ b/daemon/zerofree.c @@ -31,8 +31,7 @@ int optgroup_zerofree_available (void) { - int r = access ("/usr/sbin/zerofree", X_OK); - return r == 0; + return prog_exists ("zerofree"); } int @@ -41,7 +40,7 @@ do_zerofree (const char *device) char *err; int r; - r = command (NULL, &err, "/usr/sbin/zerofree", device, NULL); + r = command (NULL, &err, "zerofree", device, NULL); if (r == -1) { reply_with_error ("%s: %s", device, err); free (err); diff --git a/src/generator.ml b/src/generator.ml index b04bbe0..e996521 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -6145,7 +6145,7 @@ and generate_daemon_actions () pr " ret->guestfs_int_lvm_%s_list_val = NULL;\n" typ; pr "\n"; pr " r = command (&out, &err,\n"; - pr " \"/sbin/lvm\", \"%ss\",\n" typ; + pr " \"lvm\", \"%ss\",\n" typ; pr " \"-o\", lvm_%s_cols, \"--unbuffered\", \"--noheadings\",\n" typ; pr " \"--nosuffix\", \"--separator\", \",\", \"--units\", \"b\", NULL);\n"; pr " if (r == -1) {\n"; -- 1.6.6.1
Jim Meyering
2010-Mar-26 12:23 UTC
[Libguestfs] [PATCH] appliance: Set $PATH instead of hard-coding paths to binaries everywhere.
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.
Possibly Parallel Threads
- [PATCH] daemon: Rewrite prog_exists so it uses the actual PATH, not hard-coded list.
- [PATCH] daemon/RHEL: Choose correct udev settle script.
- [PATCH] daemon: Work around udevsettle issue (RHBZ#548121).
- [PATCH 0/2] daemon: Replace GUESTFSD_EXT_CMD with --print-external-commands.
- [PATCH v2 0/4] daemon: Translate device names if Linux device is unstable (RHBZ#1804207).