<div dir="ltr"><div dir="ltr"><br clear="all"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 6, 2020 at 3:57 PM Daniel P. Berrangé <<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Apr 06, 2020 at 03:49:36PM +0530, Mishal Shah wrote:<br>
> Fix libvirt build by taking care of NULL string handling<br>
<br>
What build problems are you seeing without this patch ?  Our automated<br>
CI is all working correctly and I don't think the changes make sense.<br></blockquote><div><br></div><div>../../src/openvz/openvz_driver.c: In function 'openvzDomainMigratePrepare3Params':<br>../../src/util/glibcompat.h:33:26: error: '%s' directive argument is null [-Werror=format-overflow=]<br>   33 | # define g_strdup_printf vir_g_strdup_printf<br>../../src/openvz/openvz_driver.c:2152:16: note: in expansion of macro 'g_strdup_printf'<br> 2152 |     *uri_out = g_strdup_printf("ssh://%s", hostname);<br>      |                ^~~~~~~~~~~~~~~</div><div><br></div><div>This is the error that comes on compiling build. I'm using Ubuntu 18.04 & GCC version 9.2.1<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> ---<br>
>  src/openvz/openvz_driver.c | 6 +++++-<br>
>  tests/sockettest.c         | 8 ++++++--<br>
>  2 files changed, 11 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c<br>
> index 1a189dbbe7..46f117cd00 100644<br>
> --- a/src/openvz/openvz_driver.c<br>
> +++ b/src/openvz/openvz_driver.c<br>
> @@ -2084,7 +2084,7 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn,<br>
>      virDomainDefPtr def = NULL;<br>
>      virDomainObjPtr vm = NULL;<br>
>      char *my_hostname = NULL;<br>
> -    const char *hostname = NULL;<br>
> +    char *hostname = NULL;<br>
<br>
This string is just a copy of anoter string, so shouldn't have<br>
const removed IMHO.<br>
<br>
>      virURIPtr uri = NULL;<br>
>      int ret = -1;<br>
>  <br>
> @@ -2149,6 +2149,10 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn,<br>
>          }<br>
>      }<br>
>  <br>
> +    if (hostname == NULL) {<br>
> +        goto error;<br>
> +    }<br>
<br>
I don't think this is right either - what's missing is that in the<br>
previous  if() block we should have a  "hostname = my_hostname;"<br>
assignment.<br></blockquote><div><br></div><div>Yes, I think this assignment should have been there, otherwise, the string hostname stays NULL and throws an error in the build.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
>      *uri_out = g_strdup_printf("ssh://%s", hostname);<br>
>  <br>
>      ret = 0;<br>
> diff --git a/tests/sockettest.c b/tests/sockettest.c<br>
> index 29a565de40..f0a0815b88 100644<br>
> --- a/tests/sockettest.c<br>
> +++ b/tests/sockettest.c<br>
> @@ -188,8 +188,12 @@ static int testMaskNetwork(const char *addrstr,<br>
>          return -1;<br>
>  <br>
>      if (STRNEQ(networkstr, gotnet)) {<br>
> -        VIR_FREE(gotnet);<br>
> -        fprintf(stderr, "Expected %s, got %s\n", networkstr, gotnet);<br>
> +        if (gotnet == NULL) {<br>
> +            fprintf(stderr, "Expected %s, got empty string\n", networkstr);<br>
> +        } else {<br>
> +            fprintf(stderr, "Expected %s, got %s\n", networkstr, gotnet);<br>
> +            VIR_FREE(gotnet);<br>
> +        }<br>
>          return -1;<br>
>      }<br>
<br>
This makese no sense either, since a few lines above we have<br>
<br>
    if (!(gotnet = virSocketAddrFormat(&network)))<br>
       return -1;<br>
<br>
so it is impossible for "gotnet == NULL" to be true<br></blockquote><div><br></div><div>Yes, but I think because VIR_FREE was called before the fprintf statement, gotnet became NULL and was throwing an error in the build.</div><div><br></div><div>I can make the necessary changes if required.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Regards,<br>
Daniel<br>
-- <br>
|: <a href="https://berrange.com" rel="noreferrer" target="_blank">https://berrange.com</a>      -o-    <a href="https://www.flickr.com/photos/dberrange" rel="noreferrer" target="_blank">https://www.flickr.com/photos/dberrange</a> :|<br>
|: <a href="https://libvirt.org" rel="noreferrer" target="_blank">https://libvirt.org</a>         -o-            <a href="https://fstop138.berrange.com" rel="noreferrer" target="_blank">https://fstop138.berrange.com</a> :|<br>
|: <a href="https://entangle-photo.org" rel="noreferrer" target="_blank">https://entangle-photo.org</a>    -o-    <a href="https://www.instagram.com/dberrange" rel="noreferrer" target="_blank">https://www.instagram.com/dberrange</a> :|<br>
<br>
</blockquote></div></div>