[Libguestfs] [PATCH] NFC: Cleanup iteration over fstab entries in inspect_fs_unix.c

[Richard W.M. Jones]

On Fri, Nov 25, 2011 at 01:16:54PM +0000, Matthew Booth wrote:
> Select non-comment labels using an augeas path to return the correct nodes in
> the first instance, rather than applying a regular expression to all results.
> Iterate over returned matches using a char** iterator.
> Use asprintf() to ensure the path string buffer is the correct size.
> ---
>  src/inspect_fs_unix.c |   53 +++++++++++++++++++++++-------------------------
>  1 files changed, 25 insertions(+), 28 deletions(-)
> 
> diff --git a/src/inspect_fs_unix.c b/src/inspect_fs_unix.c
> index 0fa3e83..69a2458 100644
> --- a/src/inspect_fs_unix.c
> +++ b/src/inspect_fs_unix.c
> @@ -110,7 +110,6 @@ compile_regexps (void)
>    COMPILE (re_scientific_linux_no_minor,
>             "Scientific Linux.*release (\\d+)", 0);
>    COMPILE (re_major_minor, "(\\d+)\\.(\\d+)", 0);
> -  COMPILE (re_aug_seq, "/\\d+$", 0);
>    COMPILE (re_xdev, "^/dev/(h|s|v|xv)d([a-z]+)(\\d*)$", 0);
>    COMPILE (re_cciss, "^/dev/(cciss/c\\d+d\\d+)(?:p(\\d+))?$", 0);
>    COMPILE (re_mdN, "^(/dev/md\\d+)$", 0);
> @@ -132,7 +131,6 @@ free_regexps (void)
>    pcre_free (re_scientific_linux);
>    pcre_free (re_scientific_linux_no_minor);
>    pcre_free (re_major_minor);
> -  pcre_free (re_aug_seq);
>    pcre_free (re_xdev);
>    pcre_free (re_cciss);
>    pcre_free (re_mdN);
> @@ -725,47 +723,46 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
>    Hash_table *md_map;
>    if (!map_md_devices (g, &md_map)) return -1;
>  
> -  char **lines = guestfs_aug_ls (g, "/files/etc/fstab");
> -  if (lines == NULL) goto error;
> +  char **entries = guestfs_aug_match (g, "/files/etc/fstab/*"
> +                                         "[label() != '#comment']");
> +  if (entries == NULL) goto error;
>  
> -  if (lines[0] == NULL) {
> +  if (entries[0] == NULL) {
>      error (g, _("could not parse /etc/fstab or empty file"));
>      goto error;
>    }
>  
> -  size_t i;
>    char augpath[256];
> -  for (i = 0; lines[i] != NULL; ++i) {
> -    /* Ignore comments.  Only care about sequence lines which
> -     * match m{/\d+$}.
> -     */
> -    if (match (g, lines[i], re_aug_seq)) {
> -      snprintf (augpath, sizeof augpath, "%s/spec", lines[i]);
> -      char *spec = guestfs_aug_get (g, augpath);
> -      if (spec == NULL) goto error;
> -
> -      snprintf (augpath, sizeof augpath, "%s/file", lines[i]);
> -      char *mp = guestfs_aug_get (g, augpath);
> -      if (mp == NULL) {
> -        free (spec);
> -        goto error;
> -      }
> -
> -      int r = add_fstab_entry (g, fs, spec, mp, md_map);
> +  for (char **entry = entries; *entry != NULL; entry++) {
> +    int len;
> +
> +    len = snprintf(augpath, sizeof(augpath), "%s/spec", *entry);
> +    if (len < 0 || (size_t)len >= sizeof(augpath)) g->abort_cb();
> +    char *spec = guestfs_aug_get (g, augpath);
> +    if (spec == NULL) goto error;
> +
> +    len = snprintf(augpath, sizeof(augpath), "%s/file", *entry);
> +    if (len < 0 || (size_t)len >= sizeof(augpath)) g->abort_cb();
> +    char *mp = guestfs_aug_get (g, augpath);
> +    if (mp == NULL) {
>        free (spec);
> -      free (mp);
> -
> -      if (r == -1) goto error;
> +      goto error;
>      }
> +
> +    int r = add_fstab_entry (g, fs, spec, mp, md_map);
> +    free (spec);
> +    free (mp);
> +
> +    if (r == -1) goto error;
>    }
>  
>    hash_free (md_map);
> -  guestfs___free_string_list (lines);
> +  guestfs___free_string_list (entries);
>    return 0;
>  
>  error:
>    hash_free (md_map);
> -  if (lines) guestfs___free_string_list (lines);
> +  if (entries) guestfs___free_string_list (entries);
>    return -1;
>  }
>  
> -- 
> 1.7.7.3
> 

ACK.

I'm going to run the valgrind tests on this before pushing.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw