Code cleanup. Add a goto label to simplify the code. Signed-off-by: Wanlong Gao <gaowanlong(a)cn.fujitsu.com&gt; --- src/launch.c | 29 +++++++++++------------------ 1 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/launch.c b/src/launch.c index 8eaaac8..2840767 100644 --- a/src/launch.c +++ b/src/launch.c @@ -312,18 +312,12 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, if (format && !valid_format_iface (format)) { error (g, _("%s parameter is empty or contains disallowed characters"), "format"); - free (format); - free (iface); - free (name); - return -1; + goto err_out; } if (!valid_format_iface (iface)) { error (g, _("%s parameter is empty or contains disallowed characters"), "iface"); - free (format); - free (iface); - free (name); - return -1; + goto err_out; } /* For writable files, see if we can use cache=off. This also @@ -331,20 +325,13 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, * to do the check explicitly. */ use_cache_off = readonly ? 0 : test_cache_off (g, filename); - if (use_cache_off == -1) { - free (format); - free (iface); - free (name); - return -1; - } + if (use_cache_off == -1) + goto err_out; if (readonly) { if (access (filename, R_OK) == -1) { perrorf (g, "%s", filename); - free (format); - free (iface); - free (name); - return -1; + goto err_out; } } @@ -361,6 +348,12 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, (*i)->use_cache_off = use_cache_off; return 0; + +err_out: + free (format); + free (iface); + free (name); + return -1; } int -- 1.7.8

1. Change the g->path to restore a absolute path instead of the mixed. 2. Check that if the adding drive is duplicated with the added drive. Signed-off-by: Wanlong Gao <gaowanlong(a)cn.fujitsu.com&gt; --- src/launch.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/launch.c b/src/launch.c index 2840767..86e6241 100644 --- a/src/launch.c +++ b/src/launch.c @@ -335,24 +335,33 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, } } + char *abs_path = realpath (filename, NULL); struct drive **i = &(g->drives); - while (*i != NULL) i = &((*i)->next); + while (*i != NULL) { + if (STREQ((*i)->path, abs_path)) { + error (g, _("drive %s can't be added twice"), abs_path); + goto err_out; + } + i = &((*i)->next); + } *i = safe_malloc (g, sizeof (struct drive)); (*i)->next = NULL; - (*i)->path = safe_strdup (g, filename); + (*i)->path = safe_strdup (g, abs_path); (*i)->readonly = readonly; (*i)->format = format; (*i)->iface = iface; (*i)->name = name; (*i)->use_cache_off = use_cache_off; + free (abs_path); return 0; err_out: free (format); free (iface); free (name); + free (abs_path); return -1; } -- 1.7.8

From: Wanlong Gao <gaowanlong(a)cn.fujitsu.com&gt; Move the filename's comma character checking to a wrapper. Signed-off-by: Wanlong Gao <gaowanlong(a)cn.fujitsu.com&gt; --- src/launch.c | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/launch.c b/src/launch.c index ca89b63..8eaaac8 100644 --- a/src/launch.c +++ b/src/launch.c @@ -277,6 +277,16 @@ valid_format_iface (const char *str) return 1; } +static int +check_path (guestfs_h *g, const char *filename) +{ + if (strchr (filename, ',') != NULL) { + error (g, _("filename cannot contain ',' (comma) character")); + return -1; + } + return 0; +} + int guestfs__add_drive_opts (guestfs_h *g, const char *filename, const struct guestfs_add_drive_opts_argv *optargs) @@ -287,10 +297,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, char *name; int use_cache_off; - if (strchr (filename, ',') != NULL) { - error (g, _("filename cannot contain ',' (comma) character")); + if (check_path(g, filename) == -1) return -1; - } readonly = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_READONLY_BITMASK ? optargs->readonly : 0; @@ -405,10 +413,8 @@ guestfs__add_drive_ro_with_if (guestfs_h *g, const char *filename, int guestfs__add_cdrom (guestfs_h *g, const char *filename) { - if (strchr (filename, ',') != NULL) { - error (g, _("filename cannot contain ',' (comma) character")); + if (check_path(g, filename) == -1) return -1; - } if (access (filename, F_OK) == -1) { perrorf (g, "%s", filename); -- 1.7.8