9:35 a.m.
Patches 1-4 are minor cleanups, patch 5 is the actual one that I want out
of the way before continuing down my rabbit hole of the week.
Tim Wiederhake (5):
domain_conf: Reduce scope of tmp in virDomainFeaturesDefParse
domain_conf: Reduce scope of gic_version in virDomainFeaturesDefParse
domain_conf: Reduce scope of node in virDomainFeaturesDefParse
domain_conf: Add missing break in switch
domain_conf: Fix check for hyperv stimer
src/conf/domain_conf.c | 36 ++++++++++++++----------------------
1 file changed, 14 insertions(+), 22 deletions(-)
--
2.26.2
9:35 a.m.
New subject: [libvirt PATCH 1/5] domain_conf: Reduce scope of tmp in virDomainFeaturesDefParse
Variables using `g_autofree` should not be manually VIR_FREE'd and reused.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f194909b13..1d0ef17318 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18156,7 +18156,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
{
g_autofree xmlNodePtr *nodes = NULL;
- g_autofree char *tmp = NULL;
xmlNodePtr node = NULL;
int gic_version;
size_t i;
@@ -18166,6 +18165,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
for (i = 0; i < n; i++) {
+ g_autofree char *tmp = NULL;
int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name);
if (val < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -18184,7 +18184,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
def->apic_eoi = eoi;
- VIR_FREE(tmp);
}
G_GNUC_FALLTHROUGH;
case VIR_DOMAIN_FEATURE_ACPI:
@@ -18206,7 +18205,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
tmp, virDomainFeatureTypeToString(val));
return -1;
}
- VIR_FREE(tmp);
} else {
def->features[val] = VIR_TRISTATE_SWITCH_ABSENT;
}
@@ -18225,7 +18223,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
tmp, virDomainFeatureTypeToString(val));
return -1;
}
- VIR_FREE(tmp);
} else {
def->features[val] = VIR_TRISTATE_SWITCH_ON;
}
@@ -18240,7 +18237,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
def->gic_version = gic_version;
- VIR_FREE(tmp);
}
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
@@ -18256,7 +18252,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
def->features[val] = value;
- VIR_FREE(tmp);
}
break;
@@ -18271,7 +18266,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
def->hpt_resizing = (virDomainHPTResizing) value;
- VIR_FREE(tmp);
}
if (virParseScaledValue("./features/hpt/maxpagesize",
@@ -18305,7 +18299,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
def->features[val] = value;
- VIR_FREE(tmp);
}
break;
@@ -18320,7 +18313,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
def->features[val] = value;
- VIR_FREE(tmp);
}
break;
@@ -18335,7 +18327,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
def->features[val] = value;
- VIR_FREE(tmp);
}
break;
@@ -18354,7 +18345,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
tmp, virDomainFeatureTypeToString(val));
return -1;
}
- VIR_FREE(tmp);
break;
/* coverity[dead_error_begin] */
@@ -18372,6 +18362,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
for (i = 0; i < n; i++) {
+ g_autofree char *tmp = NULL;
feature = virDomainHypervTypeFromString((const char *)nodes[i]->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -18398,7 +18389,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
- VIR_FREE(tmp);
def->hyperv_features[feature] = value;
switch ((virDomainHyperv) feature) {
@@ -18477,6 +18467,8 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
for (i = 0; i < n; i++) {
+ g_autofree char *tmp = NULL;
+
if (STRNEQ((const char *)nodes[i]->name, "direct")) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported Hyper-V stimer feature: %s"),
@@ -18498,7 +18490,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
- VIR_FREE(tmp);
def->hyperv_stimer_direct = value;
}
VIR_FREE(nodes);
@@ -18511,6 +18502,8 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
for (i = 0; i < n; i++) {
+ g_autofree char *tmp = NULL;
+
feature = virDomainKVMTypeFromString((const char *)nodes[i]->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -18539,7 +18532,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
- VIR_FREE(tmp);
def->kvm_features[feature] = value;
break;
@@ -18555,6 +18547,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
int feature;
int value;
g_autofree char *ptval = NULL;
+ g_autofree char *tmp = NULL;
if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) <
0)
return -1;
@@ -18584,7 +18577,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
return -1;
}
- VIR_FREE(tmp);
def->xen_features[feature] = value;
switch ((virDomainXen) feature) {
@@ -18638,6 +18630,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
}
if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) {
+ g_autofree char *tmp = NULL;
if ((node = virXPathNode("./features/msrs", ctxt)) == NULL)
return -1;
@@ -18654,13 +18647,13 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
tmp);
return -1;
}
- VIR_FREE(tmp);
}
if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes))
< 0)
return -1;
for (i = 0; i < n; i++) {
+ g_autofree char *tmp = NULL;
int val = virDomainProcessCapsFeatureTypeFromString((const char
*)nodes[i]->name);
if (val < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -18675,7 +18668,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
tmp, virDomainProcessCapsFeatureTypeToString(val));
return -1;
}
- VIR_FREE(tmp);
} else {
def->caps_features[val] = VIR_TRISTATE_SWITCH_ON;
}
--
2.26.2
9:35 a.m.
New subject: [libvirt PATCH 2/5] domain_conf: Reduce scope of gic_version in virDomainFeaturesDefParse
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1d0ef17318..f73814f3c7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18157,7 +18157,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
{
g_autofree xmlNodePtr *nodes = NULL;
xmlNodePtr node = NULL;
- int gic_version;
size_t i;
int n;
@@ -18230,7 +18229,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
case VIR_DOMAIN_FEATURE_GIC:
if ((tmp = virXMLPropString(nodes[i], "version"))) {
- gic_version = virGICVersionTypeFromString(tmp);
+ int gic_version = virGICVersionTypeFromString(tmp);
if (gic_version < 0 || gic_version == VIR_GIC_VERSION_NONE) {
virReportError(VIR_ERR_XML_ERROR,
_("malformed gic version: %s"), tmp);
--
2.26.2
9:35 a.m.
New subject: [libvirt PATCH 3/5] domain_conf: Reduce scope of node in virDomainFeaturesDefParse
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f73814f3c7..4bc82b69fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18156,7 +18156,6 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
{
g_autofree xmlNodePtr *nodes = NULL;
- xmlNodePtr node = NULL;
size_t i;
int n;
@@ -18356,7 +18355,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
int feature;
int value;
- node = ctxt->node;
+ xmlNodePtr node = ctxt->node;
if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, &nodes)) <
0)
return -1;
@@ -18630,6 +18629,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) {
g_autofree char *tmp = NULL;
+ xmlNodePtr node = NULL;
if ((node = virXPathNode("./features/msrs", ctxt)) == NULL)
return -1;
--
2.26.2
9:35 a.m.
New subject: [libvirt PATCH 4/5] domain_conf: Add missing break in switch
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4bc82b69fc..48fd078b90 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18449,6 +18449,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
_("HyperV vendor_id value is invalid"));
return -1;
}
+ break;
/* coverity[dead_error_begin] */
case VIR_DOMAIN_HYPERV_LAST:
--
2.26.2
9:35 a.m.
New subject: [libvirt PATCH 5/5] domain_conf: Fix check for hyperv stimer
VIR_DOMAIN_HYPERV_STIMER happens to have the same numerical value as
VIR_DOMAIN_FEATURE_HYPERV, resulting in the if-block to always being
executed when a "<hyperv>" tag is found, whether or not it actually
contained a "<stimer>" tag. This had no ill effects, as virXPathNodeSet()
would simply return 0 if that tag does not exist.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 48fd078b90..05b6cb3000 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18460,7 +18460,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
ctxt->node = node;
}
- if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) {
+ if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) {
int value;
if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt,
&nodes)) < 0)
return -1;
--
2.26.2
11:45 a.m.
New subject: [libvirt PATCH 5/5] domain_conf: Fix check for hyperv stimer
On 3/4/21 9:35 AM, Tim Wiederhake wrote:
VIR_DOMAIN_HYPERV_STIMER happens to have the same numerical value as
VIR_DOMAIN_FEATURE_HYPERV, resulting in the if-block to always being
executed when a "<hyperv>" tag is found, whether or not it actually
contained a "<stimer>" tag. This had no ill effects, as
virXPathNodeSet()
would simply return 0 if that tag does not exist.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 48fd078b90..05b6cb3000 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18460,7 +18460,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def,
ctxt->node = node;
}
- if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) {
+ if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) {
int value;
if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt,
&nodes)) < 0)
return -1;
Nice catch.
Michal
11:45 a.m.
On 3/4/21 9:35 AM, Tim Wiederhake wrote:
Patches 1-4 are minor cleanups, patch 5 is the actual one that I want
out
of the way before continuing down my rabbit hole of the week.
Tim Wiederhake (5):
domain_conf: Reduce scope of tmp in virDomainFeaturesDefParse
domain_conf: Reduce scope of gic_version in virDomainFeaturesDefParse
domain_conf: Reduce scope of node in virDomainFeaturesDefParse
domain_conf: Add missing break in switch
domain_conf: Fix check for hyperv stimer
src/conf/domain_conf.c | 36 ++++++++++++++----------------------
1 file changed, 14 insertions(+), 22 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
and pushed.
Michal
1295
days inactive
1295
days old
7 comments
2 participants
participants (2)
-
Michal Privoznik -
Tim Wiederhake