[libvirt PATCH] fix build by NULL string handling

[Mishal Shah]

On Mon, Apr 6, 2020 at 3:57 PM Daniel P. Berrangé <berrange at redhat.com>
wrote:

> On Mon, Apr 06, 2020 at 03:49:36PM +0530, Mishal Shah wrote:
> > Fix libvirt build by taking care of NULL string handling
>
> What build problems are you seeing without this patch ?  Our automated
> CI is all working correctly and I don't think the changes make sense.
>

../../src/openvz/openvz_driver.c: In function
'openvzDomainMigratePrepare3Params':
../../src/util/glibcompat.h:33:26: error: '%s' directive argument is null
[-Werror=format-overflow=]
   33 | # define g_strdup_printf vir_g_strdup_printf
../../src/openvz/openvz_driver.c:2152:16: note: in expansion of macro
'g_strdup_printf'
 2152 |     *uri_out = g_strdup_printf("ssh://%s", hostname);
      |                ^~~~~~~~~~~~~~~

This is the error that comes on compiling build. I'm using Ubuntu 18.04 &
GCC version 9.2.1


>
> > ---
> >  src/openvz/openvz_driver.c | 6 +++++-
> >  tests/sockettest.c         | 8 ++++++--
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> > index 1a189dbbe7..46f117cd00 100644
> > --- a/src/openvz/openvz_driver.c
> > +++ b/src/openvz/openvz_driver.c
> > @@ -2084,7 +2084,7 @@ openvzDomainMigratePrepare3Params(virConnectPtr
> dconn,
> >      virDomainDefPtr def = NULL;
> >      virDomainObjPtr vm = NULL;
> >      char *my_hostname = NULL;
> > -    const char *hostname = NULL;
> > +    char *hostname = NULL;
>
> This string is just a copy of anoter string, so shouldn't have
> const removed IMHO.
>
> >      virURIPtr uri = NULL;
> >      int ret = -1;
> >
> > @@ -2149,6 +2149,10 @@ openvzDomainMigratePrepare3Params(virConnectPtr
> dconn,
> >          }
> >      }
> >
> > +    if (hostname == NULL) {
> > +        goto error;
> > +    }
>
> I don't think this is right either - what's missing is that in the
> previous  if() block we should have a  "hostname = my_hostname;"
> assignment.
>

Yes, I think this assignment should have been there, otherwise, the string
hostname stays NULL and throws an error in the build.


>
> >      *uri_out = g_strdup_printf("ssh://%s", hostname);
> >
> >      ret = 0;
> > diff --git a/tests/sockettest.c b/tests/sockettest.c
> > index 29a565de40..f0a0815b88 100644
> > --- a/tests/sockettest.c
> > +++ b/tests/sockettest.c
> > @@ -188,8 +188,12 @@ static int testMaskNetwork(const char *addrstr,
> >          return -1;
> >
> >      if (STRNEQ(networkstr, gotnet)) {
> > -        VIR_FREE(gotnet);
> > -        fprintf(stderr, "Expected %s, got %s\n", networkstr, gotnet);
> > +        if (gotnet == NULL) {
> > +            fprintf(stderr, "Expected %s, got empty string\n",
> networkstr);
> > +        } else {
> > +            fprintf(stderr, "Expected %s, got %s\n", networkstr,
> gotnet);
> > +            VIR_FREE(gotnet);
> > +        }
> >          return -1;
> >      }
>
> This makese no sense either, since a few lines above we have
>
>     if (!(gotnet = virSocketAddrFormat(&network)))
>        return -1;
>
> so it is impossible for "gotnet == NULL" to be true
>

Yes, but I think because VIR_FREE was called before the fprintf statement,
gotnet became NULL and was throwing an error in the build.

I can make the necessary changes if required.


>
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200406/1ee93af1/attachment-0001.htm>