From: "Richard W.M. Jones" <rjones@redhat.com> This commit tidies up the code for the 'glob' command. It also makes the command return an error if malloc fails (previously it would just print a message but not return any error indication). --- fish/glob.c | 93 ++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 54 insertions(+), 39 deletions(-) diff --git a/fish/glob.c b/fish/glob.c index b3cfa57..88e8927 100644 --- a/fish/glob.c +++ b/fish/glob.c @@ -29,6 +29,9 @@ /* A bit tricky because in the case where there are multiple * paths we have to perform a Cartesian product. */ + +static char **expand_pathname (guestfs_h *g, const char *path); +static char **single_element_list (const char *element); static void glob_issue (char *cmd, size_t argc, char ***globs, size_t *posn, size_t *count, int *r); int @@ -69,50 +72,19 @@ run_glob (const char *cmd, size_t argc, char *argv[]) /* Only if it begins with '/' can it possibly be a globbable path. */ if (argv[i][0] == '/') { - pp = guestfs_glob_expand (g, argv[i]); - if (pp == NULL) { /* real error in glob_expand */ - fprintf (stderr, _("glob: guestfs_glob_expand call failed: %s\n"), - argv[i]); - goto error0; - } - - /* If there were no matches, then we add a single element list - * containing just the original argv[i] string. - */ - if (pp[0] == NULL) { - char **pp2; - - pp2 = realloc (pp, sizeof (char *) * 2); - if (pp2 == NULL) { - perror ("realloc"); - free (pp); - goto error0; - } - pp = pp2; - - pp[0] = strdup (argv[i]); - if (pp[0] == NULL) { - perror ("strdup"); - free (pp); - goto error0; - } - pp[1] = NULL; + pp = expand_pathname (g, argv[i]); + if (pp == NULL) { + r = -1; + goto error; } } /* Doesn't begin with '/' */ else { - pp = malloc (sizeof (char *) * 2); + pp = single_element_list (argv[i]); if (pp == NULL) { - perror ("malloc"); - goto error0; - } - pp[0] = strdup (argv[i]); - if (pp[0] == NULL) { - perror ("strdup"); - free (pp); - goto error0; + r = -1; + goto error; } - pp[1] = NULL; } globs[i] = pp; @@ -123,13 +95,56 @@ run_glob (const char *cmd, size_t argc, char *argv[]) glob_issue (argv[0], argc, globs, posn, count, &r); /* Free resources. */ - error0: + error: for (i = 1; i < argc; ++i) if (globs[i]) free_strings (globs[i]); return r; } +static char ** +expand_pathname (guestfs_h *g, const char *path) +{ + char **pp; + + pp = guestfs_glob_expand (g, path); + if (pp == NULL) { /* real error in glob_expand */ + fprintf (stderr, _("glob: guestfs_glob_expand call failed: %s\n"), path); + return NULL; + } + + if (pp[0] != NULL) + return pp; /* Return the non-empty list of matches. */ + + /* If there were no matches, then we add a single element list + * containing just the original string. + */ + free (pp); + return single_element_list (path); +} + +/* Return a single element list containing 'element'. */ +static char ** +single_element_list (const char *element) +{ + char **pp; + + pp = malloc (sizeof (char *) * 2); + if (pp == NULL) { + perror ("malloc"); + return NULL; + } + pp[0] = strdup (element); + if (pp[0] == NULL) { + perror ("strdup"); + free (pp); + return NULL; + } + pp[1] = NULL; + + return pp; +} + static void glob_issue (char *cmd, size_t argc, char ***globs, size_t *posn, size_t *count, -- 1.7.10

From: "Richard W.M. Jones" <rjones@redhat.com> For example:

<fs> glob echo /dev/* /dev/vda /dev/vda1 /dev/vda2 /dev/vda3 <fs> glob echo /dev/v*/* /dev/vg_f16x64/lv_root /dev/vg_f16x64/lv_swap

fish/glob.c | 143 +++++++++++++++++++++++++++++++++++++++- generator/generator_actions.ml | 6 +- 2 files changed, 146 insertions(+), 3 deletions(-) diff --git a/fish/glob.c b/fish/glob.c index 88e8927..075f69a 100644 --- a/fish/glob.c +++ b/fish/glob.c @@ -22,6 +22,8 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <errno.h> +#include <fnmatch.h> #include <libintl.h> #include "fish.h" @@ -31,6 +33,9 @@ */ static char **expand_pathname (guestfs_h *g, const char *path); +static char **expand_devicename (guestfs_h *g, const char *device); +static int add_strings_matching (char **pp, const char *glob, char ***ret, size_t *size_r); +static int add_string (const char *str, char ***ret, size_t *size_r); static char **single_element_list (const char *element); static void glob_issue (char *cmd, size_t argc, char ***globs, size_t *posn, size_t *count, int *r); @@ -70,8 +75,18 @@ run_glob (const char *cmd, size_t argc, char *argv[]) for (i = 1; i < argc; ++i) { char **pp; - /* Only if it begins with '/' can it possibly be a globbable path. */ - if (argv[i][0] == '/') { + /* If it begins with "/dev/" then treat it as a globbable device + * name. + */ + if (STRPREFIX (argv[i], "/dev/")) { + pp = expand_devicename (g, argv[i]); + if (pp == NULL) { + r = -1; + goto error; + } + } + /* If it begins with "/" it might be a globbable pathname. */ + else if (argv[i][0] == '/') { pp = expand_pathname (g, argv[i]); if (pp == NULL) { r = -1; @@ -123,6 +138,130 @@ expand_pathname (guestfs_h *g, const char *path) return single_element_list (path); } +/* Glob-expand device patterns, such as "/dev/sd*" (RHBZ#635971). + * + * There is no 'guestfs_glob_expand_device' function because the + * equivalent can be implemented using functions like + * 'guestfs_list_devices'. + * + * It's not immediately clear what it means to expand a pattern like + * "/dev/sd*". Should that include device name translation? Should + * the result include partitions as well as devices? + * + * Should "/dev/" + "*" return every possible device and filesystem? + * How about VGs? LVs? + * + * To solve this what we do is build up a list of every device, + * partition, etc., then glob against that list. + * + * Notes for future work (XXX): + * - This doesn't handle device name translation. It wouldn't be + * too hard to add. + * - Could have an API function for returning all device-like things. + */ +static char ** +expand_devicename (guestfs_h *g, const char *device) +{ + char **pp = NULL; + char **ret = NULL; + size_t size = 0; + + pp = guestfs_list_devices (g); + if (pp == NULL) goto error; + if (add_strings_matching (pp, device, &ret, &size) == -1) goto error; + free_strings (pp); + + pp = guestfs_list_partitions (g); + if (pp == NULL) goto error; + if (add_strings_matching (pp, device, &ret, &size) == -1) goto error; + free_strings (pp); + + pp = guestfs_list_md_devices (g); + if (pp == NULL) goto error; + if (add_strings_matching (pp, device, &ret, &size) == -1) goto error; + free_strings (pp); + + if (feature_available (g, "lvm2")) { + pp = guestfs_lvs (g); + if (pp == NULL) goto error; + if (add_strings_matching (pp, device, &ret, &size) == -1) goto error; + free_strings (pp); + pp = NULL; + } + + /* None matched? Add the original glob pattern. */ + if (ret == NULL) + ret = single_element_list (device); + return ret; + + error: + if (pp) + free_strings (pp); + if (ret) + free_strings (ret); + + return NULL; +} + +/* Using fnmatch, find strings in the list 'pp' which match pattern + * 'glob'. Add strings which match to the 'ret' array. '*size_r' is + * the current size of the 'ret' array, which is updated with the new + * size. + */ +static int +add_strings_matching (char **pp, const char *glob, + char ***ret, size_t *size_r) +{ + size_t i; + int r; + + for (i = 0; pp[i] != NULL; ++i) { + errno = 0; + r = fnmatch (glob, pp[i], FNM_PATHNAME); + if (r == 0) { /* matches - add it */ + if (add_string (pp[i], ret, size_r) == -1) + return -1; + } + else if (r != FNM_NOMATCH) { /* error */ + /* I checked the glibc impl and it returns random negative + * numbers for errors. It doesn't always set errno. Do our + * best here to record the error state. + */ + fprintf (stderr, "glob: fnmatch: error (r = %d, errno = %d)\n", + r, errno); + return -1; + } + } + + return 0; +} + +static int +add_string (const char *str, char ***ret, size_t *size_r) +{ + char **new_ret = *ret; + size_t size = *size_r; + + new_ret = realloc (new_ret, (size + 2) * (sizeof (char *))); + if (!new_ret) { + perror ("realloc"); + return -1; + } + *ret = new_ret; + + new_ret[size] = strdup (str); + if (new_ret[size] == NULL) { + perror ("strdup"); + return -1; + } + + size++; + new_ret[size] = NULL; + *size_r = size; + + return 0; +} + /* Return a single element list containing 'element'. */ static char ** single_element_list (const char *element) diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index ace46cb..6948dab 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -3700,7 +3700,11 @@ If no paths match, then this returns an empty list It is just a wrapper around the C L<glob(3)> function with flags C<GLOB_MARK|GLOB_BRACE>. -See that manual page for more details."); +See that manual page for more details. + +Notice that there is no equivalent command for expanding a device +name (eg. C</dev/sd*>). Use C<guestfs_list_devices>, +C<guestfs_list_partitions> etc functions instead."); ("scrub_device", (RErr, [Device "device"], []), 114, [Optional "scrub"], [InitNone, Always, TestRun ( (* use /dev/sdc because it's smaller *) -- 1.7.10