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(a)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