[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 1/1] Switch from YAJL to Jansson



On Thursday, 23 November 2017 15:00:12 CET Martin Kletzander wrote:
> On Thu, Nov 23, 2017 at 02:23:52PM +0100, Pino Toscano wrote:
> >While YAJL mostly works fine, it did not see any active development in
> >the latest 3 years.  OTOH, Jansson is another JSON C implementation,
> >with a very liberal license, and a much nicer API.
> >
> >Hence, switch all of libguestfs from YAJL to Jansson:
> >- configure checks, and buildsystem in general
> >- packages pulled in the appliance
> >- actual implementations
> >- contrib scripts
> >- documentation
> >
> >This also makes use of the better APIs available (e.g. json_object_get,
> >json_array_foreach, and json_object_foreach).  This does not change the
> >API of our OCaml Yajl module.
> >
> >The only behaviour change is that Jansson by default accepts only
> >objects and arrays as input (even though it has a flag to make it accept
> >anything).  Since this seems a good idea anyway, and as it is the RFC
> >behaviour, then change one of the yajl_tests checks accordingly.
> >---
> > appliance/packagelist.in            |  13 ++--
> > builder/Makefile.am                 |   4 +-
> > builder/yajl-c.c                    |  66 ++++++++++-----------
> > builder/yajl_tests.ml               |   4 +-
> > contrib/p2v/aux-scripts/do-build.sh |   8 +--
> > contrib/p2v/build-p2v-iso.sh        |   3 +-
> > daemon/Makefile.am                  |   4 +-
> > daemon/ldm.c                        | 115 ++++++++++++++++--------------------
> > docs/guestfs-building.pod           |   2 +-
> > lib/Makefile.am                     |   4 +-
> > lib/info.c                          | 113 ++++++++++++++---------------------
> > lib/qemu.c                          |  64 +++++++-------------
> > m4/guestfs-libraries.m4             |   4 +-
> > 13 files changed, 170 insertions(+), 234 deletions(-)
> >
> 
> This is nice, just one idea below that might be useful to you.
> 
> >diff --git a/lib/info.c b/lib/info.c
> >index f7378adfd..ada4cf03c 100644
> >--- a/lib/info.c
> >+++ b/lib/info.c
> >@@ -38,53 +38,46 @@
> > #include <sys/resource.h>
> > #endif
> >
> >-#include <yajl/yajl_tree.h>
> >+#include <jansson.h>
> >
> > #include "guestfs.h"
> > #include "guestfs-internal.h"
> > #include "guestfs-internal-actions.h"
> >
> > #ifdef HAVE_ATTRIBUTE_CLEANUP
> >-#define CLEANUP_YAJL_TREE_FREE __attribute__((cleanup(cleanup_yajl_tree_free)))
> >+#define CLEANUP_JSON_T_DECREF __attribute__((cleanup(cleanup_json_t_decref)))
> >
> > static void
> >-cleanup_yajl_tree_free (void *ptr)
> >+cleanup_json_t_decref (void *ptr)
> > {
> >-  yajl_tree_free (* (yajl_val *) ptr);
> >+  json_decref (* (json_t **) ptr);
> > }
> >
> 
> Looks like you don't need to do this, there already is 'json_auto_t' type that
> has attribute cleanup with json_decref already.

json_auto_t was introduced in Jansson 2.9, and I'd like to maintain
compatibility at least with Jansson 2.7 (shipped in Ubuntu 16.04, the
latest LTS, for example).

Nice idea though.

-- 
Pino Toscano

Attachment: signature.asc
Description: This is a digitally signed message part.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]