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

[libvirt] [PATCH] xenDaemonDomainSetAutostart: avoid appearance of impropriety



Coverity noticed that of the 13 uses of sexpr_lookup,
only this one was unchecked, and here, the result is dereferenced.
It happens to be a false positive, due to the preceding sexpr_node
check, but worth fixing: sexpr_node is just a very thin wrapper
around sexpr_lookup so there's little justification for looking
up the same string twice.

>From a9ab34214cf9d247d39731563dcc70b8f1dc73b5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering redhat com>
Date: Wed, 17 Feb 2010 22:14:25 +0100
Subject: [PATCH] xenDaemonDomainSetAutostart: avoid appearance of impropriety

* src/xen/xend_internal.c (xenDaemonDomainSetAutostart): Rewrite to
avoid dereferencing the result of sexpr_lookup.  While in this
particular case, it was guaranteed never to be NULL, due to the
preceding "if sexpr_node(...)" guard, it's cleaner to skip the
sexpr_node call altogether, and also saves a lookup.
---
 src/xen/xend_internal.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 88923c8..1f8b106 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4383,7 +4383,6 @@ xenDaemonDomainSetAutostart(virDomainPtr domain,
                             int autostart)
 {
     struct sexpr *root, *autonode;
-    const char *autostr;
     char buf[4096];
     int ret = -1;
     xenUnifiedPrivatePtr priv;
@@ -4408,16 +4407,17 @@ xenDaemonDomainSetAutostart(virDomainPtr domain,
         return (-1);
     }

-    autostr = sexpr_node(root, "domain/on_xend_start");
-    if (autostr) {
-        if (!STREQ(autostr, "ignore") && !STREQ(autostr, "start")) {
+    autonode = sexpr_lookup(root, "domain/on_xend_start");
+    if (autonode) {
+        const char *val = (autonode->u.s.car->kind == SEXPR_VALUE
+                           ? autonode->u.s.car->u.value : NULL);
+        if (!STREQ(val, "ignore") && !STREQ(val, "start")) {
             virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR,
                          "%s", _("unexpected value from on_xend_start"));
             goto error;
         }

         // Change the autostart value in place, then define the new sexpr
-        autonode = sexpr_lookup(root, "domain/on_xend_start");
         VIR_FREE(autonode->u.s.car->u.value);
         autonode->u.s.car->u.value = (autostart ? strdup("start")
                                                 : strdup("ignore"));
--
1.7.0.219.g6bb57


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