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

[Guido Günther <agx@xxxxxxxxxxx>]

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@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