<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 20, 2018 at 12:12 AM, John Ferlan <span dir="ltr"><<a href="mailto:jferlan@redhat.com" target="_blank">jferlan@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 09/18/2018 02:48 PM, Fabiano Fidêncio wrote:<br>
> Signed-off-by: Fabiano Fidêncio <<a href="mailto:fidencio@redhat.com">fidencio@redhat.com</a>><br>
> ---<br>
> src/xenconfig/xen_common.c | 23 +++++++++++------------<br>
> 1 file changed, 11 insertions(+), 12 deletions(-)<br>
> <br>
> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c<br>
> index a6e77a9250..786c276c99 100644<br>
> --- a/src/xenconfig/xen_common.c<br>
> +++ b/src/xenconfig/xen_common.c<br>
> @@ -759,9 +759,11 @@ xenParseVfb(virConfPtr conf, virDomainDefPtr def)<br>
> static int<br>
> xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)<br>
> {<br>
> + char **serials = NULL;<br>
> const char *str;<br>
> virConfValuePtr value = NULL;<br>
> virDomainChrDefPtr chr = NULL;<br>
> + int ret = -1;<br>
> <br>
> if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {<br>
> if (xenConfigGetString(conf, "parallel", &str, NULL) < 0)<br>
<br>
</span>Heh heh - this is what I noted earlier - @str isn't going to be FREE'd.<br>
<span class=""><br>
> @@ -782,7 +784,8 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)<br>
> <br>
> /* Try to get the list of values to support multiple serial ports */<br>
> value = virConfGetValue(conf, "serial");<br>
> - if (value && value->type == VIR_CONF_LIST) {<br>
> + if (virConfGetValueStringList(<wbr>conf, "serial", false, &serials) == 1) {<br>
<br>
</span>Well this is similar to the previous two except that you kept @value<br>
here and that line probably should have been deleted as would the value<br>
= value->next below and the variable itself as it's unused... At least<br>
it shouldn't run off the end of the list...<br>
<div><div class="h5"><br>
> + char **entries;<br>
> int portnum = -1;<br>
> <br>
> if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) {<br>
> @@ -791,18 +794,12 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)<br>
> goto cleanup;<br>
> }<br>
> <br>
> - value = value->list;<br>
> - while (value) {<br>
> - char *port = NULL;<br>
> + for (entries = serials; *entries; entries++) {<br>
> + char *port = *entries;<br>
> <br>
> - if ((value->type != VIR_CONF_STRING) || (value->str == NULL))<br>
> - goto cleanup;<br>
> - port = value->str;<br>
> portnum++;<br>
> - if (STREQ(port, "none")) {<br>
> - value = value->next;<br>
> + if (STREQ(port, "none"))<br>
> continue;<br>
> - }<br>
> <br>
> if (!(chr = xenParseSxprChar(port, NULL)))<br>
> goto cleanup;<br>
> @@ -827,6 +824,7 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)<br>
> chr->target.port = 0;<br>
> def->serials[0] = chr;<br>
> def->nserials++;<br>
> + chr = NULL;<br>
<br>
</div></div>Hmmm... unrelated but equally bad bug. This would require it's own<br>
separate patch.<br></blockquote><div><br></div><div>After re-working the patches this change is not needed anymore! \o/<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
John<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
> }<br>
> }<br>
> } else {<br>
> @@ -840,11 +838,12 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)<br>
> def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_<wbr>TYPE_XEN;<br>
> }<br>
> <br>
> - return 0;<br>
> + ret = 0;<br>
> <br>
> cleanup:<br>
> virDomainChrDefFree(chr);<br>
> - return -1;<br>
> + virStringListFree(serials);<br>
> + return ret;<br>
> }<br>
> <br>
> <br>
> <br>
</div></div></blockquote></div><br></div></div>