On Tue, Jul 31, 2018 at 05:57:21PM +0200, Andrea Bolognani wrote: > On Tue, 2018-07-31 at 15:55 +0100, Daniel P. Berrangé wrote: > [...] > > +# We dlopen() it so need an explicit dep > > +Requires: libjansson.so.4()(64bit) > > Wouldn't requiring jansson be better here? I don't think many > people are running libvirt on 32-bit machines these days, but > the (64bit) part still looks kinda weird. Yes, ok. > > @@ -264,7 +266,6 @@ libvirt_util_la_CFLAGS = \ > > $(NULL) > > libvirt_util_la_LIBADD = \ > > $(CAPNG_LIBS) \ > > - $(JANSSON_LIBS) \ > > You missed a couple of instances of $(JANSSON_LIBS), notably the > one used to link the NSS plugin against it ;) > > > @@ -1524,6 +1525,9 @@ virJSONValueFromString(const char *jsonstring) > > size_t flags = JSON_REJECT_DUPLICATES | > > JSON_DECODE_ANY; > > > > + if (virJSONInitialize() < 0) > > + return NULL; > > Shouldn't we rather virReportError() here? Or does it not matter > since we do that inside virJSONInitialize() already? That method is already reporting errors when needed. > The lines above trigger a syntax-check failure; there are other > issues as well, please make sure you address all of them. Yeah done in v2. > > > + fprintf(stderr, "Resolve %s to %p\n", #name, name ## _ptr); \ > > I guess the fprintf() is a leftover from development: please drop > it to avoid stuff like Opps. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
