Re: [PATCH 1/2] xen: add error handling to UUID parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 07, 2011 at 12:18:51PM +0200, Guido Günther wrote:
> On Thu, Oct 06, 2011 at 04:33:49PM -0600, Eric Blake wrote:
> > On 10/06/2011 11:36 AM, Guido Günther wrote:
> > >>These three errors should probably all be changed away from
> > >>VIR_ERR_INTERNAL_ERROR into something more useful, but I'm not sure
> > >>whether that would be VIR_ERR_CONF_SYNTAX,
> > >>VIR_ERR_CONFIG_UNSUPPORTED, or something else.
> > >
> > >I was uncertain about the error codes too, that's why I kept
> > >VIR_ERR_INTERNAL_ERROR as used by other functions in this file. I'd
> > >opt for VIR_ERR_CONF_SYNTAX.
> > >  -- Guido
> > 
> > Yeah, for this particular case, where we are parsing a xen conf
> > file, CONF_SYNTAX is the best fit.
> > 
> > >
> > >>
> > >>ACK to the concept, once we decide on the correct error code.
> > 
> > I think we've decided.
> (Hopefully) final version attached. I've left the first error as
> INTERNAL_ERROR since this it's there do detect false function
> parameters. O.k. to apply?

Ping? 
 -- Guido

> Cheers,
>  -- Guido

> >From 573d2a5a575dd551b92e603201bbae7bb5fa1733 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Guido=20G=C3=BCnther?= <agx@xxxxxxxxxxx>
> Date: Thu, 6 Oct 2011 12:42:39 +0200
> Subject: [PATCH] xen: add error handling to UUID parsing
> 
> otherwise a missing UUID in a domain config just shows:
> 
> error: An error occured, but the cause is unknown
> 
> Now we have:
> 
> error: configuration file syntax error: config value uuid was missing
> ---
>  src/xenxs/xen_xm.c |   37 +++++++++++++++++++++++++++----------
>  1 files changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> index 70facf7..ff173d8 100644
> --- a/src/xenxs/xen_xm.c
> +++ b/src/xenxs/xen_xm.c
> @@ -174,21 +174,38 @@ static int xenXMConfigCopyStringOpt(virConfPtr conf,
>  /* Convenience method to grab a string UUID from the config file object */
>  static int xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char *uuid) {
>      virConfValuePtr val;
> -    if (!uuid || !name || !conf)
> -        return (-1);
> +
> +    if (!uuid || !name || !conf) {
> +        XENXS_ERROR(VIR_ERR_INVALID_ARG,
> +                   _("Arguments must be non null"));
> +        return -1;
> +    }
> +
>      if (!(val = virConfGetValue(conf, name))) {
> -        return (-1);
> +        XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> +                   _("config value %s was missing"), name);
> +        return -1;
>      }
>  
> -    if (val->type != VIR_CONF_STRING)
> -        return (-1);
> -    if (!val->str)
> -        return (-1);
> +    if (val->type != VIR_CONF_STRING) {
> +        XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> +                   _("config value %s not a string"), name);
> +        return -1;
> +    }
> +
> +    if (!val->str) {
> +        XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> +                   _("%s can't be empty"), name);
> +        return -1;
> +    }
>  
> -    if (virUUIDParse(val->str, uuid) < 0)
> -        return (-1);
> +    if (virUUIDParse(val->str, uuid) < 0) {
> +        XENXS_ERROR(VIR_ERR_CONF_SYNTAX,
> +                   _("%s not parseable"), val->str);
> +        return -1;
> +    }
>  
> -    return (0);
> +    return 0;
>  }
>  
>  #define MAX_VFB 1024
> -- 
> 1.7.6.3
> 

> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]