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