[libvirt] [PATCH] util: avoid symbol clash between json libraries

[Andrea Bolognani]

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.

> @@ -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?

[...]
> +#define LOAD(name) \
> +    do { \
> +        if (!(name ## _ptr = dlsym(handle, #name))) {                  \
> +            virReportError(VIR_ERR_NO_SUPPORT,                          \
> +                           _("missing symbol '%s' in libjansson.so.4: %s"), #name, dlerror()); \
> +            goto error;                                                 \
> +        } \

The lines above trigger a syntax-check failure; there are other
issues as well, please make sure you address all of them.

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

  $ ping debian
  Resolve json_array to 0x7fdcf77b2fa0
  Resolve json_array_append_new to 0x7fdcf77b3e60
  ...
  Resolve json_string_value to 0x7fdcf77b3200
  Resolve json_true to 0x7fdcf77b36c0
  PING debian (192.168.124.124) 56(84) bytes of data.
  64 bytes from 192.168.124.124 (192.168.124.124): icmp_seq=1 ttl=64 time=0.164 ms

[...]
> +int virJSONInitialize(void) {
> +    return virJSONJanssonInitialize();
> +}

Please format this as

  int
  virJSONInitialize(void) {
      ...

Same for all the stubs below.


Everything else looks pretty much as sane as a crazy hack like this
could possibly ever look :) so with the above addressed

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

with the caveats that you should probably wait for at least another
ACK before pushing it, and additionally we should *really* bump up
the release date and let this sit on master for more than a couple
of days...

-- 
Andrea Bolognani / Red Hat / Virtualization