[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part XII
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainAudioPulseAudioParse: Use virXMLProp* virDomainAudioDef: Change type of "type" to virDomainAudioType virDomainAudioDef: Change type of "sdl.driver" to virDomainAudioSDLDriver virDomainAudioDefParseXML: Use virXMLProp* virDomainAudioDefParseXML: Don't ignore return value of virDomainAudio*Parse() virDomainIOMMUDefParseXML: Use virXMLProp* virStorageAdapterParseXML: Use g_autofree virStorageAdapterFCHost: Change type of "type" to virStorageAdapterType virStorageAdapterParseXML: Use virXMLProp* virDomainDeviceSpaprVioAddressParseXML: Use virXMLProp* src/bhyve/bhyve_command.c | 2 +- src/conf/device_conf.c | 14 +-- src/conf/domain_conf.c | 202 +++++++++++--------------------- src/conf/domain_conf.h | 4 +- src/conf/storage_adapter_conf.c | 38 ++---- src/conf/storage_adapter_conf.h | 2 +- src/qemu/qemu_command.c | 4 +- src/qemu/qemu_validate.c | 2 +- 8 files changed, 97 insertions(+), 171 deletions(-) -- 2.26.3
This strictens the parser to disallow negative values (interpreted as `UINT_MAX + value + 1`) for attribute `latency`. Allowing negative numbers to be interpreted this way makes no sense for this attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b3ed3a9c5a..942d6f269a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13092,18 +13092,12 @@ static int virDomainAudioPulseAudioParse(virDomainAudioIOPulseAudio *def, xmlNodePtr node) { - g_autofree char *latency = virXMLPropString(node, "latency"); - def->name = virXMLPropString(node, "name"); def->streamName = virXMLPropString(node, "streamName"); - if (latency && - virStrToLong_ui(latency, NULL, 10, - &def->latency) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("cannot parse 'latency' value '%s'"), latency); + if (virXMLPropUInt(node, "latency", 10, VIR_XML_PROP_NONE, + &def->latency) < 0) return -1; - } return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/bhyve/bhyve_command.c | 2 +- src/conf/domain_conf.c | 18 ++++++++++-------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_validate.c | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index f8e0ce5123..ab9d3026cc 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -524,7 +524,7 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, virCommandAddArg(cmd, "-s"); if (audio) { - switch ((virDomainAudioType) audio->type) { + switch (audio->type) { case VIR_DOMAIN_AUDIO_TYPE_OSS: if (virDomainAudioIOCommonIsSet(&audio->input) || virDomainAudioIOCommonIsSet(&audio->output)) { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 942d6f269a..758f699c2c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2922,7 +2922,7 @@ virDomainAudioDefFree(virDomainAudioDef *def) if (!def) return; - switch ((virDomainAudioType) def->type) { + switch (def->type) { case VIR_DOMAIN_AUDIO_TYPE_NONE: break; @@ -13123,24 +13123,26 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, virDomainAudioDef *def; VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autofree char *tmp = NULL; - g_autofree char *type = NULL; + g_autofree char *typestr = NULL; + int type; xmlNodePtr inputNode, outputNode; def = g_new0(virDomainAudioDef, 1); ctxt->node = node; - type = virXMLPropString(node, "type"); - if (!type) { + typestr = virXMLPropString(node, "type"); + if (!typestr) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing audio 'type' attribute")); goto error; } - if ((def->type = virDomainAudioTypeTypeFromString(type)) < 0) { + if ((type = virDomainAudioTypeTypeFromString(typestr)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown audio type '%s'"), type); + _("unknown audio type '%s'"), typestr); goto error; } + def->type = type; tmp = virXMLPropString(node, "id"); if (!tmp) { @@ -13163,7 +13165,7 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, if (outputNode && virDomainAudioCommonParse(&def->output, outputNode, ctxt) < 0) goto error; - switch ((virDomainAudioType) def->type) { + switch (def->type) { case VIR_DOMAIN_AUDIO_TYPE_NONE: break; @@ -25465,7 +25467,7 @@ virDomainAudioDefFormat(virBuffer *buf, virBufferAsprintf(buf, "<audio id='%d' type='%s'", def->id, type); - switch ((virDomainAudioType)def->type) { + switch (def->type) { case VIR_DOMAIN_AUDIO_TYPE_NONE: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cf8481f1f6..462c61a807 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1543,7 +1543,7 @@ struct _virDomainAudioIOSDL { }; struct _virDomainAudioDef { - int type; + virDomainAudioType type; unsigned int id; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d6c5308ef0..dcc060bde9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7670,7 +7670,7 @@ qemuBuildAudioCommandLineArg(virCommand *cmd, qemuBuildAudioCommonArg(&buf, "in", &def->input); qemuBuildAudioCommonArg(&buf, "out", &def->output); - switch ((virDomainAudioType)def->type) { + switch (def->type) { case VIR_DOMAIN_AUDIO_TYPE_NONE: break; @@ -7859,7 +7859,7 @@ qemuBuildAudioCommandLineEnv(virCommand *cmd, qemuBuildAudioCommonEnv(cmd, "QEMU_AUDIO_ADC_", &audio->input); qemuBuildAudioCommonEnv(cmd, "QEMU_AUDIO_DAC_", &audio->output); - switch ((virDomainAudioType)audio->type) { + switch (audio->type) { case VIR_DOMAIN_AUDIO_TYPE_NONE: break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 141203f979..e6ddb43113 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4223,7 +4223,7 @@ qemuValidateDomainDeviceDefAudio(virDomainAudioDef *audio, } } - switch ((virDomainAudioType)audio->type) { + switch (audio->type) { case VIR_DOMAIN_AUDIO_TYPE_NONE: break; -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 17 +++++++++-------- src/conf/domain_conf.h | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 758f699c2c..9e6719265f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13236,15 +13236,16 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, break; case VIR_DOMAIN_AUDIO_TYPE_SDL: { - g_autofree char *driver = virXMLPropString(node, "driver"); - if (driver && - (def->backend.sdl.driver = - virDomainAudioSDLDriverTypeFromString(driver)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown SDL driver '%s'"), driver); - goto error; + g_autofree char *driverstr = virXMLPropString(node, "driver"); + int driver; + if (driverstr) { + if ((driver = virDomainAudioSDLDriverTypeFromString(driverstr)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown SDL driver '%s'"), driverstr); + goto error; + } + def->backend.sdl.driver = driver; } - if (inputNode) virDomainAudioSDLParse(&def->backend.sdl.input, inputNode); if (outputNode) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 462c61a807..fab856a5c7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1578,7 +1578,7 @@ struct _virDomainAudioDef { struct { virDomainAudioIOSDL input; virDomainAudioIOSDL output; - int driver; /* virDomainAudioSDLDriver */ + virDomainAudioSDLDriver driver; } sdl; struct { char *path; -- 2.26.3
On 5/19/21 4:10 PM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 17 +++++++++-------- src/conf/domain_conf.h | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 758f699c2c..9e6719265f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13236,15 +13236,16 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, break;
case VIR_DOMAIN_AUDIO_TYPE_SDL: { - g_autofree char *driver = virXMLPropString(node, "driver"); - if (driver && - (def->backend.sdl.driver = - virDomainAudioSDLDriverTypeFromString(driver)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown SDL driver '%s'"), driver); - goto error; + g_autofree char *driverstr = virXMLPropString(node, "driver"); + int driver; + if (driverstr) { + if ((driver = virDomainAudioSDLDriverTypeFromString(driverstr)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown SDL driver '%s'"), driverstr);
Misaligned argument.
+ goto error; + } + def->backend.sdl.driver = driver; } - if (inputNode) virDomainAudioSDLParse(&def->backend.sdl.input, inputNode); if (outputNode) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 462c61a807..fab856a5c7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1578,7 +1578,7 @@ struct _virDomainAudioDef { struct { virDomainAudioIOSDL input; virDomainAudioIOSDL output; - int driver; /* virDomainAudioSDLDriver */ + virDomainAudioSDLDriver driver; } sdl; struct { char *path;
Michal
This strictens the parser to disallow negative values (interpreted as `UINT_MAX + value + 1`) for attribute `id`. Allowing negative numbers to be interpreted this way makes no sense for this attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 74 ++++++++++++------------------------------ 1 file changed, 21 insertions(+), 53 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9e6719265f..2142e45fd5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13122,40 +13122,18 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, { virDomainAudioDef *def; VIR_XPATH_NODE_AUTORESTORE(ctxt) - g_autofree char *tmp = NULL; - g_autofree char *typestr = NULL; - int type; xmlNodePtr inputNode, outputNode; def = g_new0(virDomainAudioDef, 1); ctxt->node = node; - typestr = virXMLPropString(node, "type"); - if (!typestr) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing audio 'type' attribute")); - goto error; - } - - if ((type = virDomainAudioTypeTypeFromString(typestr)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown audio type '%s'"), typestr); + if (virXMLPropEnum(node, "type", virDomainAudioTypeTypeFromString, + VIR_XML_PROP_REQUIRED, &def->type) < 0) goto error; - } - def->type = type; - tmp = virXMLPropString(node, "id"); - if (!tmp) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("missing audio 'id' attribute")); - goto error; - } - if (virStrToLong_ui(tmp, NULL, 10, &def->id) < 0 || - def->id == 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid audio 'id' value '%s'"), tmp); + if (virXMLPropUInt(node, "id", 10, VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &def->id) < 0) goto error; - } inputNode = virXPathNode("./input", ctxt); outputNode = virXPathNode("./output", ctxt); @@ -13191,29 +13169,25 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, break; case VIR_DOMAIN_AUDIO_TYPE_OSS: { - g_autofree char *tryMMap = virXMLPropString(node, "tryMMap"); - g_autofree char *exclusive = virXMLPropString(node, "exclusive"); - g_autofree char *dspPolicy = virXMLPropString(node, "dspPolicy"); + int dspPolicySet; - if (tryMMap && ((def->backend.oss.tryMMap = - virTristateBoolTypeFromString(tryMMap)) <= 0)) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown 'tryMMap' value '%s'"), tryMMap); + if (virXMLPropTristateBool(node, "tryMMap", VIR_XML_PROP_NONE, + &def->backend.oss.tryMMap) < 0) goto error; - } - if (exclusive && ((def->backend.oss.exclusive = - virTristateBoolTypeFromString(exclusive)) <= 0)) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown 'exclusive' value '%s'"), exclusive); + if (virXMLPropTristateBool(node, "exclusive", VIR_XML_PROP_NONE, + &def->backend.oss.exclusive) < 0) + goto error; + + if ((dspPolicySet = virXMLPropInt(node, "dspPolicy", 10, VIR_XML_PROP_NONE, + &def->backend.oss.dspPolicy, 0)) < 0) goto error; - } - if (dspPolicy) { - if (virStrToLong_i(dspPolicy, NULL, 10, - &def->backend.oss.dspPolicy) < 0) { + if (dspPolicySet != 0) { + if (def->backend.oss.dspPolicy < 0) { virReportError(VIR_ERR_XML_ERROR, - _("cannot parse 'dspPolicy' value '%s'"), dspPolicy); + _("cannot parse 'dspPolicy' value '%i'"), + def->backend.oss.dspPolicy); goto error; } def->backend.oss.dspPolicySet = true; @@ -13236,16 +13210,10 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, break; case VIR_DOMAIN_AUDIO_TYPE_SDL: { - g_autofree char *driverstr = virXMLPropString(node, "driver"); - int driver; - if (driverstr) { - if ((driver = virDomainAudioSDLDriverTypeFromString(driverstr)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown SDL driver '%s'"), driverstr); - goto error; - } - def->backend.sdl.driver = driver; - } + if (virXMLPropEnum(node, "driver", virDomainAudioSDLDriverTypeFromString, + VIR_XML_PROP_NONZERO, &def->backend.sdl.driver) < 0) + goto error; + if (inputNode) virDomainAudioSDLParse(&def->backend.sdl.input, inputNode); if (outputNode) -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 50 +++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2142e45fd5..1350c46039 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13155,17 +13155,21 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, break; case VIR_DOMAIN_AUDIO_TYPE_COREAUDIO: - if (inputNode) - virDomainAudioCoreAudioParse(&def->backend.coreaudio.input, inputNode); - if (outputNode) - virDomainAudioCoreAudioParse(&def->backend.coreaudio.output, outputNode); + if (inputNode && + virDomainAudioCoreAudioParse(&def->backend.coreaudio.input, inputNode) < 0) + goto error; + if (outputNode && + virDomainAudioCoreAudioParse(&def->backend.coreaudio.output, outputNode) < 0) + goto error; break; case VIR_DOMAIN_AUDIO_TYPE_JACK: - if (inputNode) - virDomainAudioJackParse(&def->backend.jack.input, inputNode); - if (outputNode) - virDomainAudioJackParse(&def->backend.jack.output, outputNode); + if (inputNode && + virDomainAudioJackParse(&def->backend.jack.input, inputNode) < 0) + goto error; + if (outputNode && + virDomainAudioJackParse(&def->backend.jack.output, outputNode) < 0) + goto error; break; case VIR_DOMAIN_AUDIO_TYPE_OSS: { @@ -13193,20 +13197,24 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, def->backend.oss.dspPolicySet = true; } - if (inputNode) - virDomainAudioOSSParse(&def->backend.oss.input, inputNode); - if (outputNode) - virDomainAudioOSSParse(&def->backend.oss.output, outputNode); + if (inputNode && + virDomainAudioOSSParse(&def->backend.oss.input, inputNode) < 0) + goto error; + if (outputNode && + virDomainAudioOSSParse(&def->backend.oss.output, outputNode) < 0) + goto error; break; } case VIR_DOMAIN_AUDIO_TYPE_PULSEAUDIO: def->backend.pulseaudio.serverName = virXMLPropString(node, "serverName"); - if (inputNode) - virDomainAudioPulseAudioParse(&def->backend.pulseaudio.input, inputNode); - if (outputNode) - virDomainAudioPulseAudioParse(&def->backend.pulseaudio.output, outputNode); + if (inputNode && + virDomainAudioPulseAudioParse(&def->backend.pulseaudio.input, inputNode) < 0) + goto error; + if (outputNode && + virDomainAudioPulseAudioParse(&def->backend.pulseaudio.output, outputNode) < 0) + goto error; break; case VIR_DOMAIN_AUDIO_TYPE_SDL: { @@ -13214,10 +13222,12 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt G_GNUC_UNUSED, VIR_XML_PROP_NONZERO, &def->backend.sdl.driver) < 0) goto error; - if (inputNode) - virDomainAudioSDLParse(&def->backend.sdl.input, inputNode); - if (outputNode) - virDomainAudioSDLParse(&def->backend.sdl.output, outputNode); + if (inputNode && + virDomainAudioSDLParse(&def->backend.sdl.input, inputNode) < 0) + goto error; + if (outputNode && + virDomainAudioSDLParse(&def->backend.sdl.output, outputNode) < 0) + goto error; break; } -- 2.26.3
This strictens the parser to disallow negative values (interpreted as `UINT_MAX + value + 1`) for attribute `aw_bits`. Allowing negative numbers to be interpreted this way makes no sense for this attribute. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 69 +++++++++++------------------------------- 1 file changed, 17 insertions(+), 52 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1350c46039..e35c38caa3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14805,71 +14805,36 @@ virDomainIOMMUDefParseXML(xmlNodePtr node, { VIR_XPATH_NODE_AUTORESTORE(ctxt) xmlNodePtr driver; - int val; - g_autofree char *tmp = NULL; g_autofree virDomainIOMMUDef *iommu = NULL; ctxt->node = node; iommu = g_new0(virDomainIOMMUDef, 1); - if (!(tmp = virXMLPropString(node, "model"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing model for IOMMU device")); + if (virXMLPropEnum(node, "model", virDomainIOMMUModelTypeFromString, + VIR_XML_PROP_REQUIRED, &iommu->model) < 0) return NULL; - } - - if ((val = virDomainIOMMUModelTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown IOMMU model: %s"), tmp); - return NULL; - } - - iommu->model = val; if ((driver = virXPathNode("./driver", ctxt))) { - VIR_FREE(tmp); - if ((tmp = virXMLPropString(driver, "intremap"))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value: %s"), tmp); - return NULL; - } - iommu->intremap = val; - } + if (virXMLPropTristateSwitch(driver, "intremap", VIR_XML_PROP_NONE, + &iommu->intremap) < 0) + return NULL; - VIR_FREE(tmp); - if ((tmp = virXMLPropString(driver, "caching_mode"))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown caching_mode value: %s"), tmp); - return NULL; - } - iommu->caching_mode = val; - } - VIR_FREE(tmp); - if ((tmp = virXMLPropString(driver, "iotlb"))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown iotlb value: %s"), tmp); - return NULL; - } - iommu->iotlb = val; - } + if (virXMLPropTristateSwitch(driver, "caching_mode", VIR_XML_PROP_NONE, + &iommu->caching_mode) < 0) + return NULL; - VIR_FREE(tmp); - if ((tmp = virXMLPropString(driver, "eim"))) { - if ((val = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown eim value: %s"), tmp); - return NULL; - } - iommu->eim = val; - } + if (virXMLPropTristateSwitch(driver, "iotlb", VIR_XML_PROP_NONE, + &iommu->iotlb) < 0) + return NULL; - VIR_FREE(tmp); - if ((tmp = virXMLPropString(driver, "aw_bits"))) { - if (virStrToLong_ui(tmp, NULL, 10, &iommu->aw_bits) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("unknown aw_bits value: %s"), tmp); - return NULL; - } - } + if (virXMLPropTristateSwitch(driver, "eim", VIR_XML_PROP_NONE, + &iommu->eim) < 0) + return NULL; + if (virXMLPropUInt(driver, "aw_bits", 10, VIR_XML_PROP_NONE, + &iommu->aw_bits) < 0) + return NULL; } return g_steal_pointer(&iommu); -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/storage_adapter_conf.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c index 6b5a58e1e7..717d00dc4a 100644 --- a/src/conf/storage_adapter_conf.c +++ b/src/conf/storage_adapter_conf.c @@ -168,9 +168,8 @@ virStorageAdapterParseXML(virStorageAdapter *adapter, xmlNodePtr node, xmlXPathContextPtr ctxt) { - int ret = -1; VIR_XPATH_NODE_AUTORESTORE(ctxt) - char *adapter_type = NULL; + g_autofree char *adapter_type = NULL; ctxt->node = node; @@ -180,27 +179,23 @@ virStorageAdapterParseXML(virStorageAdapter *adapter, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown pool adapter type '%s'"), adapter_type); - goto cleanup; + return -1; } if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) && (virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0) - goto cleanup; + return -1; if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) && (virStorageAdapterParseXMLSCSIHost(node, ctxt, &adapter->data.scsi_host)) < 0) - goto cleanup; + return -1; } else { if (virStorageAdapterParseXMLLegacy(node, ctxt, adapter) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(adapter_type); - return ret; + return 0; } -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/storage_adapter_conf.c | 5 +++-- src/conf/storage_adapter_conf.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c index 717d00dc4a..a834eee81f 100644 --- a/src/conf/storage_adapter_conf.c +++ b/src/conf/storage_adapter_conf.c @@ -168,19 +168,20 @@ virStorageAdapterParseXML(virStorageAdapter *adapter, xmlNodePtr node, xmlXPathContextPtr ctxt) { + int type; VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autofree char *adapter_type = NULL; ctxt->node = node; if ((adapter_type = virXMLPropString(node, "type"))) { - if ((adapter->type = - virStorageAdapterTypeFromString(adapter_type)) <= 0) { + if ((type = virStorageAdapterTypeFromString(adapter_type)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown pool adapter type '%s'"), adapter_type); return -1; } + adapter->type = type; if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) && (virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0) diff --git a/src/conf/storage_adapter_conf.h b/src/conf/storage_adapter_conf.h index e6d9c864cd..1f0e74631e 100644 --- a/src/conf/storage_adapter_conf.h +++ b/src/conf/storage_adapter_conf.h @@ -54,7 +54,7 @@ struct _virStorageAdapterFCHost { typedef struct _virStorageAdapter virStorageAdapter; struct _virStorageAdapter { - int type; /* virStorageAdapterType */ + virStorageAdapterType type; union { virStorageAdapterSCSIHost scsi_host; -- 2.26.3
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/storage_adapter_conf.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c index a834eee81f..e93b26f06f 100644 --- a/src/conf/storage_adapter_conf.c +++ b/src/conf/storage_adapter_conf.c @@ -170,31 +170,23 @@ virStorageAdapterParseXML(virStorageAdapter *adapter, { int type; VIR_XPATH_NODE_AUTORESTORE(ctxt) - g_autofree char *adapter_type = NULL; ctxt->node = node; - if ((adapter_type = virXMLPropString(node, "type"))) { - if ((type = virStorageAdapterTypeFromString(adapter_type)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown pool adapter type '%s'"), - adapter_type); + if ((type = virXMLPropEnum(node, "type", virStorageAdapterTypeFromString, VIR_XML_PROP_NONZERO, &adapter->type)) < 0) return -1; - } - adapter->type = type; - if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) && - (virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0) - return -1; + if (type == 0) + return virStorageAdapterParseXMLLegacy(node, ctxt, adapter); - if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) && - (virStorageAdapterParseXMLSCSIHost(node, ctxt, - &adapter->data.scsi_host)) < 0) - return -1; - } else { - if (virStorageAdapterParseXMLLegacy(node, ctxt, adapter) < 0) - return -1; - } + if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) && + (virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0) + return -1; + + if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) && + (virStorageAdapterParseXMLSCSIHost(node, ctxt, + &adapter->data.scsi_host)) < 0) + return -1; return 0; } -- 2.26.3
On 5/19/21 4:10 PM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/storage_adapter_conf.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c index a834eee81f..e93b26f06f 100644 --- a/src/conf/storage_adapter_conf.c +++ b/src/conf/storage_adapter_conf.c @@ -170,31 +170,23 @@ virStorageAdapterParseXML(virStorageAdapter *adapter, { int type; VIR_XPATH_NODE_AUTORESTORE(ctxt) - g_autofree char *adapter_type = NULL;
ctxt->node = node;
- if ((adapter_type = virXMLPropString(node, "type"))) { - if ((type = virStorageAdapterTypeFromString(adapter_type)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Unknown pool adapter type '%s'"), - adapter_type); + if ((type = virXMLPropEnum(node, "type", virStorageAdapterTypeFromString, VIR_XML_PROP_NONZERO, &adapter->type)) < 0)
Whoa, that's a very long line :-)
return -1; - } - adapter->type = type;
- if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) && - (virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0) - return -1; + if (type == 0) + return virStorageAdapterParseXMLLegacy(node, ctxt, adapter);
- if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) && - (virStorageAdapterParseXMLSCSIHost(node, ctxt, - &adapter->data.scsi_host)) < 0) - return -1; - } else { - if (virStorageAdapterParseXMLLegacy(node, ctxt, adapter) < 0) - return -1; - } + if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) && + (virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0) + return -1; + + if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) && + (virStorageAdapterParseXMLSCSIHost(node, ctxt, + &adapter->data.scsi_host)) < 0) + return -1;
return 0; }
Michal
This strictens the parser to disallow negative values (interpreted as `ULLONG_MAX + value + 1`) for attribute `reg`. Allowing negative numbers to be interpreted this way makes no sense for this attribute, as it refers to a 32 bit address space. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/device_conf.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 034f072df4..e587d90c59 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -417,19 +417,17 @@ int virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node, virDomainDeviceSpaprVioAddress *addr) { - g_autofree char *reg = virXMLPropString(node, "reg"); + int reg; memset(addr, 0, sizeof(*addr)); - if (reg) { - if (virStrToLong_ull(reg, NULL, 16, &addr->reg) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'reg' attribute")); - return -1; - } + if ((reg = virXMLPropULongLong(node, "reg", 16, VIR_XML_PROP_NONE, + &addr->reg)) < 0) + return -1; + if (reg != 0) addr->has_reg = true; - } + return 0; } -- 2.26.3
On 5/19/21 4:10 PM, Tim Wiederhake wrote:
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Tim Wiederhake (10): virDomainAudioPulseAudioParse: Use virXMLProp* virDomainAudioDef: Change type of "type" to virDomainAudioType virDomainAudioDef: Change type of "sdl.driver" to virDomainAudioSDLDriver virDomainAudioDefParseXML: Use virXMLProp* virDomainAudioDefParseXML: Don't ignore return value of virDomainAudio*Parse() virDomainIOMMUDefParseXML: Use virXMLProp* virStorageAdapterParseXML: Use g_autofree virStorageAdapterFCHost: Change type of "type" to virStorageAdapterType virStorageAdapterParseXML: Use virXMLProp* virDomainDeviceSpaprVioAddressParseXML: Use virXMLProp*
src/bhyve/bhyve_command.c | 2 +- src/conf/device_conf.c | 14 +-- src/conf/domain_conf.c | 202 +++++++++++--------------------- src/conf/domain_conf.h | 4 +- src/conf/storage_adapter_conf.c | 38 ++---- src/conf/storage_adapter_conf.h | 2 +- src/qemu/qemu_command.c | 4 +- src/qemu/qemu_validate.c | 2 +- 8 files changed, 97 insertions(+), 171 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Michal Prívozník -
Tim Wiederhake