[PATCH] NFC: Cleanup iteration over fstab entries in inspect_fs_unix.c
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..27a27e5 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
On 11/25/2011 12:55 PM, 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(-)
There's a whitespace issue in this patch. Fixed in a sec. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
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
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
I pushed it, but it's been rebased and I made some other changes. Have a look here: http://git.annexia.org/?p=libguestfs.git;a=commitdiff;h=35c8e4fa7f5c90cc8371... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
participants (2)
-
Matthew Booth -
Richard W.M. Jones