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

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?

Cheers, -- Guido

>From 573d2a5a575dd551b92e603201bbae7bb5fa1733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= <agx(a)sigxcpu.org&gt; 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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list