<div dir="ltr"><div dir="ltr"></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 6, 2020 at 4:39 PM Daniel P. Berrangé <<a href="mailto:berrange@redhat.com">berrange@redhat.com</a>> wrote: </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 04:16:48PM +0530, Mishal Shah wrote: > On Mon, Apr 6, 2020 at 3:57 PM Daniel P. Berrangé <<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>> > 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 Have you made any local changes to libvirt code, or are you passing any special flags to configure, or custom env ? We test the build on Ubuntu 18.04 and don't see these warnings eg this is the latest build log <a href="https://gitlab.com/libvirt/libvirt/-/jobs/497900982" rel="noreferrer" target="_blank">https://gitlab.com/libvirt/libvirt/-/jobs/497900982</a></blockquote><div> </div><div>No, I haven't passed any special flags. The issue is caused because of the GCC version and not Ubuntu version IMO. </div><div> </div><div>I found two links to support:</div><div><ul><li>Similar issue: <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1670377">https://bugzilla.redhat.com/show_bug.cgi?id=1670377</a> </li><li>Fix: <a href="https://gitlab.gnome.org/GNOME/glib/-/merge_requests/626">https://gitlab.gnome.org/GNOME/glib/-/merge_requests/626</a></li></ul></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > > > > > > > --- > > > 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. Oh yes, the order needs to be reversed Regards, Daniel -- |: <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> :| |: <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> :| |: <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> :| </blockquote></div></div>