<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>