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