Re: [libvirt] [libvirt PATCH v6 15/15] xen_common: Change xenParseCharDev to using virConfGetValueStringList
On 09/18/2018 02:48 PM, Fabiano Fidêncio wrote:
Signed-off-by: Fabiano Fidêncio <fidencio(a)redhat.com>
---
src/xenconfig/xen_common.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index a6e77a9250..786c276c99 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -759,9 +759,11 @@ xenParseVfb(virConfPtr conf, virDomainDefPtr def)
static int
xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)
{
+ char **serials = NULL;
const char *str;
virConfValuePtr value = NULL;
virDomainChrDefPtr chr = NULL;
+ int ret = -1;
if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
if (xenConfigGetString(conf, "parallel", &str, NULL) < 0)
Heh heh - this is what I noted earlier - @str isn't going to be FREE'd.
@@ -782,7 +784,8 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr
def, const char *nativeFormat)
/* Try to get the list of values to support multiple serial ports */
value = virConfGetValue(conf, "serial");
- if (value && value->type == VIR_CONF_LIST) {
+ if (virConfGetValueStringList(conf, "serial", false, &serials) ==
1) {
Well this is similar to the previous two except that you kept @value
here and that line probably should have been deleted as would the value
= value->next below and the variable itself as it's unused... At least
it shouldn't run off the end of the list...
+ char **entries;
int portnum = -1;
if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) {
@@ -791,18 +794,12 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char
*nativeFormat)
goto cleanup;
}
- value = value->list;
- while (value) {
- char *port = NULL;
+ for (entries = serials; *entries; entries++) {
+ char *port = *entries;
- if ((value->type != VIR_CONF_STRING) || (value->str == NULL))
- goto cleanup;
- port = value->str;
portnum++;
- if (STREQ(port, "none")) {
- value = value->next;
+ if (STREQ(port, "none"))
continue;
- }
if (!(chr = xenParseSxprChar(port, NULL)))
goto cleanup;
@@ -827,6 +824,7 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char
*nativeFormat)
chr->target.port = 0;
def->serials[0] = chr;
def->nserials++;
+ chr = NULL;
Hmmm... unrelated but equally bad bug. This would require it's own
separate patch.
John
}
}
} else {
@@ -840,11 +838,12 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char
*nativeFormat)
def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
}
- return 0;
+ ret = 0;
cleanup:
virDomainChrDefFree(chr);
- return -1;
+ virStringListFree(serials);
+ return ret;
}