Wanlong Gao
2012-Jan-09 07:22 UTC
[Libguestfs] [PATCH 1/3] launch: move the filename checking to a wrapper
Move the filename's comma character checking to a wrapper. Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com> --- 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)) 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)) return -1; - } if (access (filename, F_OK) == -1) { perrorf (g, "%s", filename); -- 1.7.8
Wanlong Gao
2012-Jan-09 07:22 UTC
[Libguestfs] [PATCH 2/3] launch: add a goto label when add_drive error
Code cleanup. Add a goto label to simplify the code. Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com> --- 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
Wanlong Gao
2012-Jan-09 07:22 UTC
[Libguestfs] [PATCH 3/3] launch: don't add a drive twice
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 at cn.fujitsu.com> --- 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
Richard W.M. Jones
2012-Jan-09 10:22 UTC
[Libguestfs] [PATCH 1/3] launch: move the filename checking to a wrapper
On Mon, Jan 09, 2012 at 03:22:41PM +0800, Wanlong Gao wrote:> Move the filename's comma character checking to a wrapper. > > Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com> > --- > 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;Almost every checking function like this returns -1 on error (and 0 for OK).> + } > + 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))So this would have to change also to: 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)) > return -1; > - } > > if (access (filename, F_OK) == -1) { > perrorf (g, "%s", filename); > -- > 1.7.8But basically this patch is fine. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
Maybe Matching Threads
- [PATCH 0/4] libguestfs cannot open disk images which are symlinks to files that contain ':' (colon) character (RHBZ#812092).
- [PATCH 0/5] Assorted patches to add virtio-scsi support.
- [PATCH v2 0/9]
- Re: [PATCH v2 03/18] New API parameter: Add discard parameter to guestfs_add_drive_opts.
- [PATCH] kinit/kinit.c