Pino Toscano
2017-Nov-23 13:23 UTC
[Libguestfs] [PATCH 0/1] RFC: switch from YAJL to Jansson
Hi, recently, there was a discussion in the development list of libvirt on switching to a different JSON library than YAJL [1]. Since we use YAJL, and the points there IMHO apply to libguestfs as well, I decided to give a try in switching to Jansson [2]. The result IMHO is nice, with the additional APIs of Jansson that simplify some of our code. Unlike with YAJL, I did not set a minimum required version, as I did not use any feature in newer versions (simply because there was no need for them), so even older versions should work. Of course the test suite works for me, although it does not cover the LDM APIs. What do you think? [1] https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html [2] http://www.digip.org/jansson/ Pino Toscano (1): Switch from YAJL to Jansson 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(-) -- 2.13.6
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(-)
diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index 9e5080029..78aedad0b 100644
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -35,6 +35,7 @@ ifelse(REDHAT,1,
hivex
iproute
iputils
+ jansson
kernel
libcap
libldm
@@ -51,7 +52,6 @@ ifelse(REDHAT,1,
systemd dnl for /sbin/reboot and udevd
vim-minimal
xz
- yajl
yara
zfs-fuse
)
@@ -81,12 +81,12 @@ dnl iproute has been renamed to iproute2
libc-bin
libcap2
libhivex0
+ libjansson4
libpcre3
libsystemd0
libsystemd-id128-0
libsystemd-journal0
libtirpc1
- libyajl2
libyara3
linux-image
dnl syslinux 'suggests' mtools, but in reality it's a hard
dependency:
@@ -115,6 +115,7 @@ ifelse(ARCHLINUX,1,
hivex
iproute2
iputils
+ jansson
libcap
libtirpc
linux
@@ -130,7 +131,6 @@ ifelse(ARCHLINUX,1,
systemd
vim
xz
- yajl
yara
)
@@ -152,9 +152,9 @@ ifelse(SUSE,1,
iputils
libcap2
libhivex0
+ libjansson4
libselinux1
libtirpc3
- libyajl2
libyara3
mkisofs
ntfsprogs
@@ -176,6 +176,7 @@ ifelse(FRUGALWARE,1,
hfsplus
iproute2
iputils
+ jansson
kernel
libcap
libtirpc
@@ -187,7 +188,6 @@ ifelse(FRUGALWARE,1,
systemd
vim
xz
- yajl
xfsprogs-acl
xfsprogs-attr
gptfdisk
@@ -208,9 +208,9 @@ ifelse(MAGEIA,1,
iproute2
iputils
libcap
+ libjansson4
libldm
libtirpc
- libyajl
dnl syslinux uses mtools without depending on it
mtools
nilfs-utils
@@ -223,7 +223,6 @@ ifelse(MAGEIA,1,
systemd /* for /sbin/reboot and udevd */
vim-minimal
xz
- yajl
)
acl
diff --git a/builder/Makefile.am b/builder/Makefile.am
index 979726bff..7eeb9d85c 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -155,7 +155,7 @@ virt_builder_CFLAGS = \
$(LIBLZMA_CFLAGS) \
$(LIBTINFO_CFLAGS) \
$(LIBXML2_CFLAGS) \
- $(YAJL_CFLAGS)
+ $(JANSSON_CFLAGS)
BOBJECTS = $(SOURCES_ML:.ml=.cmo)
XOBJECTS = $(BOBJECTS:.cmo=.cmx)
@@ -209,7 +209,7 @@ OCAMLCLIBS = \
$(LIBCRYPT_LIBS) \
$(LIBLZMA_LIBS) \
$(LIBXML2_LIBS) \
- $(YAJL_LIBS) \
+ $(JANSSON_LIBS) \
$(LIBINTL) \
-lgnu
diff --git a/builder/yajl-c.c b/builder/yajl-c.c
index 3c2402e42..03fbced6e 100644
--- a/builder/yajl-c.c
+++ b/builder/yajl-c.c
@@ -23,24 +23,17 @@
#include <caml/memory.h>
#include <caml/mlvalues.h>
-#include <yajl/yajl_tree.h>
+#include <jansson.h>
#include <stdio.h>
#include <string.h>
-/* GCC can't work out that the YAJL_IS_<foo> test is sufficient to
- * ensure that YAJL_GET_<foo> later doesn't return NULL.
- */
-#if defined(__GNUC__) && __GNUC__ >= 6 /* gcc >= 6 */
-#pragma GCC diagnostic ignored "-Wnull-dereference"
-#endif
-
#define Val_none (Val_int (0))
value virt_builder_yajl_tree_parse (value stringv);
static value
-convert_yajl_value (yajl_val val, int level)
+convert_json_t (json_t *val, int level)
{
CAMLparam0 ();
CAMLlocal4 (rv, lv, v, sv);
@@ -48,46 +41,51 @@ convert_yajl_value (yajl_val val, int level)
if (level > 20)
caml_invalid_argument ("too many levels of object/array
nesting");
- if (YAJL_IS_OBJECT (val)) {
- const size_t len = YAJL_GET_OBJECT(val)->len;
+ if (json_is_object (val)) {
+ const size_t len = json_object_size (val);
size_t i;
+ const char *key;
+ json_t *jvalue;
rv = caml_alloc (1, 3);
lv = caml_alloc_tuple (len);
- for (i = 0; i < len; ++i) {
+ i = 0;
+ json_object_foreach (val, key, jvalue) {
v = caml_alloc_tuple (2);
- sv = caml_copy_string (YAJL_GET_OBJECT(val)->keys[i]);
+ sv = caml_copy_string (key);
Store_field (v, 0, sv);
- sv = convert_yajl_value (YAJL_GET_OBJECT(val)->values[i], level + 1);
+ sv = convert_json_t (jvalue, level + 1);
Store_field (v, 1, sv);
Store_field (lv, i, v);
+ ++i;
}
Store_field (rv, 0, lv);
- } else if (YAJL_IS_ARRAY (val)) {
- const size_t len = YAJL_GET_ARRAY(val)->len;
+ } else if (json_is_array (val)) {
+ const size_t len = json_array_size (val);
size_t i;
+ json_t *jvalue;
rv = caml_alloc (1, 4);
lv = caml_alloc_tuple (len);
- for (i = 0; i < len; ++i) {
- v = convert_yajl_value (YAJL_GET_ARRAY(val)->values[i], level + 1);
+ json_array_foreach (val, i, jvalue) {
+ v = convert_json_t (jvalue, level + 1);
Store_field (lv, i, v);
}
Store_field (rv, 0, lv);
- } else if (YAJL_IS_STRING (val)) {
+ } else if (json_is_string (val)) {
rv = caml_alloc (1, 0);
- v = caml_copy_string (YAJL_GET_STRING(val));
+ v = caml_copy_string (json_string_value (val));
Store_field (rv, 0, v);
- } else if (YAJL_IS_DOUBLE (val)) {
+ } else if (json_is_real (val)) {
rv = caml_alloc (1, 2);
- v = caml_copy_double (YAJL_GET_DOUBLE(val));
+ v = caml_copy_double (json_real_value (val));
Store_field (rv, 0, v);
- } else if (YAJL_IS_INTEGER (val)) {
+ } else if (json_is_integer (val)) {
rv = caml_alloc (1, 1);
- v = caml_copy_int64 (YAJL_GET_INTEGER(val));
+ v = caml_copy_int64 (json_integer_value (val));
Store_field (rv, 0, v);
- } else if (YAJL_IS_TRUE (val)) {
+ } else if (json_is_true (val)) {
rv = caml_alloc (1, 5);
Store_field (rv, 0, Val_true);
- } else if (YAJL_IS_FALSE (val)) {
+ } else if (json_is_false (val)) {
rv = caml_alloc (1, 5);
Store_field (rv, 0, Val_false);
} else
@@ -101,21 +99,21 @@ virt_builder_yajl_tree_parse (value stringv)
{
CAMLparam1 (stringv);
CAMLlocal1 (rv);
- yajl_val tree;
- char error_buf[256];
+ json_t *tree;
+ json_error_t err;
- tree = yajl_tree_parse (String_val (stringv), error_buf, sizeof error_buf);
+ tree = json_loads (String_val (stringv), 0, &err);
if (tree == NULL) {
- char buf[256 + sizeof error_buf];
- if (strlen (error_buf) > 0)
- snprintf (buf, sizeof buf, "JSON parse error: %s", error_buf);
+ char buf[256 + JSON_ERROR_TEXT_LENGTH];
+ if (strlen (err.text) > 0)
+ snprintf (buf, sizeof buf, "JSON parse error: %s", err.text);
else
snprintf (buf, sizeof buf, "unknown JSON parse error");
caml_invalid_argument (buf);
}
- rv = convert_yajl_value (tree, 1);
- yajl_tree_free (tree);
+ rv = convert_json_t (tree, 1);
+ json_decref (tree);
CAMLreturn (rv);
}
diff --git a/builder/yajl_tests.ml b/builder/yajl_tests.ml
index f5a44f2fa..3d780d4f1 100644
--- a/builder/yajl_tests.ml
+++ b/builder/yajl_tests.ml
@@ -82,6 +82,7 @@ let test_tree_parse_invalid ctx
assert_raises_invalid_argument "";
assert_raises_invalid_argument "invalid";
assert_raises_invalid_argument ":5";
+ assert_raises_invalid_argument "\"foo\"";
(* Nested objects/arrays. *)
let str = "[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]" in
@@ -93,9 +94,6 @@ let test_tree_parse_basic ctx let value = yajl_tree_parse
"{}" in
assert_is_object value;
- let value = yajl_tree_parse "\"foo\"" in
- assert_is_string "foo" value;
-
let value = yajl_tree_parse "[]" in
assert_is_array value
diff --git a/contrib/p2v/aux-scripts/do-build.sh
b/contrib/p2v/aux-scripts/do-build.sh
index 5edb53d0e..dd6424bb4 100644
--- a/contrib/p2v/aux-scripts/do-build.sh
+++ b/contrib/p2v/aux-scripts/do-build.sh
@@ -53,8 +53,8 @@ case $osversion in
# This just forces configure to ignore these missing dependencies.
export LIBTINFO_CFLAGS=-D_GNU_SOURCE
export LIBTINFO_LIBS=-lncurses
- export YAJL_CFLAGS=-D_GNU_SOURCE
- export YAJL_LIBS=-lyajl
+ export JANSSON_CFLAGS=-D_GNU_SOURCE
+ export JANSSON_LIBS=-ljansson
# Remove some unsupported flags that the configure script hard codes.
sed -i -e 's/-fno-strict-overflow//' configure
sed -i -e 's/-Wno-strict-overflow//' configure
@@ -66,8 +66,8 @@ case $osversion in
# This just forces configure to ignore these missing dependencies.
export LIBTINFO_CFLAGS=-D_GNU_SOURCE
export LIBTINFO_LIBS=-lncurses
- export YAJL_CFLAGS=-D_GNU_SOURCE
- export YAJL_LIBS=-lyajl
+ export JANSSON_CFLAGS=-D_GNU_SOURCE
+ export JANSSON_LIBS=-ljansson
;;
esac
diff --git a/contrib/p2v/build-p2v-iso.sh b/contrib/p2v/build-p2v-iso.sh
index c80a1b134..ae25cebc8 100755
--- a/contrib/p2v/build-p2v-iso.sh
+++ b/contrib/p2v/build-p2v-iso.sh
@@ -86,7 +86,6 @@ done
# Various hacks for different versions of RHEL.
if=virtio
netdev=virtio-net-pci
-pkgs="$pkgs,yajl-devel"
declare -a epel
case $osversion in
rhel-5.*|centos-5.*)
@@ -105,10 +104,12 @@ case $osversion in
rhel-6.*|centos-6.*)
epel[0]="--run-command"
epel[1]="yum install -y --nogpgcheck
https://dl.fedoraproject.org/pub/epel/epel-release-latest-6.noarch.rpm"
+ pkgs="$pkgs,jansson-devel"
;;
rhel-7.*|centos-7.*)
epel[0]="--run-command"
epel[1]="yum install -y --nogpgcheck
https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm"
+ pkgs="$pkgs,jansson-devel"
;;
esac
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 27630d2bc..cc8154c88 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -201,7 +201,7 @@ guestfsd_LDADD = \
camldaemon.o \
$(ACL_LIBS) \
$(CAP_LIBS) \
- $(YAJL_LIBS) \
+ $(JANSSON_LIBS) \
$(SELINUX_LIBS) \
$(AUGEAS_LIBS) \
$(HIVEX_LIBS) \
@@ -239,7 +239,7 @@ guestfsd_CFLAGS = \
$(AUGEAS_CFLAGS) \
$(HIVEX_CFLAGS) \
$(SD_JOURNAL_CFLAGS) \
- $(YAJL_CFLAGS) \
+ $(JANSSON_CFLAGS) \
$(PCRE_CFLAGS)
# Parts of the daemon are written in OCaml. These are linked into a
diff --git a/daemon/ldm.c b/daemon/ldm.c
index 2f4d2aef3..be4fb9701 100644
--- a/daemon/ldm.c
+++ b/daemon/ldm.c
@@ -25,19 +25,12 @@
#include <sys/stat.h>
#include <string.h>
-#include <yajl/yajl_tree.h>
+#include <jansson.h>
#include "daemon.h"
#include "actions.h"
#include "optgroups.h"
-/* GCC can't work out that the YAJL_IS_<foo> test is sufficient to
- * ensure that YAJL_GET_<foo> later doesn't return NULL.
- */
-#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 60000 /* gcc >= 6
*/
-#pragma GCC diagnostic ignored "-Wnull-dereference"
-#endif
-
int
optgroup_ldm_available (void)
{
@@ -72,44 +65,42 @@ do_ldmtool_remove_all (void)
return 0;
}
-static yajl_val
+static json_t *
parse_json (const char *json, const char *func)
{
- yajl_val tree;
- char parse_error[1024];
+ json_t *tree;
+ json_error_t err;
if (verbose)
fprintf (stderr, "%s: parsing json: %s\n", func, json);
- tree = yajl_tree_parse (json, parse_error, sizeof parse_error);
+ tree = json_loads (json, 0, &err);
if (tree == NULL) {
reply_with_error ("parse error: %s",
- strlen (parse_error) ? parse_error : "unknown
error");
+ strlen (err.text) ? err.text : "unknown
error");
return NULL;
}
- /* Caller should free this by doing 'yajl_tree_free (tree);'. */
+ /* Caller should free this by doing 'json_decref (tree);'. */
return tree;
}
#define TYPE_ERROR ((char **) -1)
static char **
-json_value_to_string_list (yajl_val node)
+json_value_to_string_list (json_t *node)
{
CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (strs);
- yajl_val n;
- size_t i, len;
+ json_t *n;
+ size_t i;
- if (! YAJL_IS_ARRAY (node))
+ if (!json_is_array (node))
return TYPE_ERROR;
- len = YAJL_GET_ARRAY (node)->len;
- for (i = 0; i < len; ++i) {
- n = YAJL_GET_ARRAY (node)->values[i];
- if (! YAJL_IS_STRING (n))
+ json_array_foreach (node, i, n) {
+ if (!json_is_string (n))
return TYPE_ERROR;
- if (add_string (&strs, YAJL_GET_STRING (n)) == -1)
+ if (add_string (&strs, json_string_value (n)) == -1)
return NULL;
}
if (end_stringsbuf (&strs) == -1)
@@ -123,14 +114,14 @@ parse_json_get_string_list (const char *json,
const char *func, const char *cmd)
{
char **ret;
- yajl_val tree = NULL;
+ json_t *tree = NULL;
tree = parse_json (json, func);
if (tree == NULL)
return NULL;
ret = json_value_to_string_list (tree);
- yajl_tree_free (tree);
+ json_decref (tree);
if (ret == TYPE_ERROR) {
reply_with_error ("output of '%s' was not a JSON array of
strings", cmd);
return NULL;
@@ -144,43 +135,40 @@ static char *
parse_json_get_object_string (const char *json, const char *key, int flags,
const char *func, const char *cmd)
{
- char *str, *ret;
- yajl_val tree = NULL, node;
- size_t i, len;
+ const char *str;
+ char *ret;
+ json_t *tree = NULL, *node;
tree = parse_json (json, func);
if (tree == NULL)
return NULL;
- if (! YAJL_IS_OBJECT (tree))
+ if (!json_is_object (tree))
+ goto bad_type;
+
+ node = json_object_get (tree, key);
+ if (node == NULL)
goto bad_type;
- len = YAJL_GET_OBJECT (tree)->len;
- for (i = 0; i < len; ++i) {
- if (STREQ (YAJL_GET_OBJECT (tree)->keys[i], key)) {
- node = YAJL_GET_OBJECT (tree)->values[i];
-
- if ((flags & GET_STRING_NULL_TO_EMPTY) && YAJL_IS_NULL
(node))
- ret = strdup ("");
- else {
- str = YAJL_GET_STRING (node);
- if (str == NULL)
- goto bad_type;
- ret = strdup (str);
- }
- if (ret == NULL)
- reply_with_perror ("strdup");
-
- yajl_tree_free (tree);
-
- return ret;
- }
+ if ((flags & GET_STRING_NULL_TO_EMPTY) && json_is_null (node))
+ ret = strdup ("");
+ else {
+ str = json_string_value (node);
+ if (str == NULL)
+ goto bad_type;
+ ret = strndup (str, json_string_length (node));
}
+ if (ret == NULL)
+ reply_with_perror ("strdup");
+
+ json_decref (tree);
+
+ return ret;
bad_type:
reply_with_error ("output of '%s' was not a JSON object "
"containing a key '%s' of type string",
cmd, key);
- yajl_tree_free (tree);
+ json_decref (tree);
return NULL;
}
@@ -189,33 +177,30 @@ parse_json_get_object_string_list (const char *json, const
char *key,
const char *func, const char *cmd)
{
char **ret;
- yajl_val tree, node;
- size_t i, len;
+ json_t *tree, *node;
tree = parse_json (json, func);
if (tree == NULL)
return NULL;
- if (! YAJL_IS_OBJECT (tree))
+ if (!json_is_object (tree))
goto bad_type;
- len = YAJL_GET_OBJECT (tree)->len;
- for (i = 0; i < len; ++i) {
- if (STREQ (YAJL_GET_OBJECT (tree)->keys[i], key)) {
- node = YAJL_GET_OBJECT (tree)->values[i];
- ret = json_value_to_string_list (node);
- if (ret == TYPE_ERROR)
- goto bad_type;
- yajl_tree_free (tree);
- return ret;
- }
- }
+ node = json_object_get (tree, key);
+ if (node == NULL)
+ goto bad_type;
+
+ ret = json_value_to_string_list (node);
+ if (ret == TYPE_ERROR)
+ goto bad_type;
+ json_decref (tree);
+ return ret;
bad_type:
reply_with_error ("output of '%s' was not a JSON object "
"containing a key '%s' of type array of
strings",
cmd, key);
- yajl_tree_free (tree);
+ json_decref (tree);
return NULL;
}
diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index a75983b83..e1b54b967 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -169,7 +169,7 @@ I<Required>.
I<Required>.
-=item yajl E<ge> 2.0.4
+=item Jansson
I<Required>.
diff --git a/lib/Makefile.am b/lib/Makefile.am
index a1d8ee70e..6e97a4844 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -141,7 +141,7 @@ libguestfs_la_CFLAGS = \
$(PCRE_CFLAGS) \
$(LIBVIRT_CFLAGS) \
$(LIBXML2_CFLAGS) \
- $(YAJL_CFLAGS)
+ $(JANSSON_CFLAGS)
libguestfs_la_LIBADD = \
../common/errnostring/liberrnostring.la \
@@ -152,7 +152,7 @@ libguestfs_la_LIBADD = \
$(PCRE_LIBS) \
$(LIBVIRT_LIBS) $(LIBXML2_LIBS) \
$(SELINUX_LIBS) \
- $(YAJL_LIBS) \
+ $(JANSSON_LIBS) \
../gnulib/lib/libgnu.la \
$(GETADDRINFO_LIB) \
$(HOSTENT_LIB) \
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);
}
#else
-#define CLEANUP_YAJL_TREE_FREE
+#define CLEANUP_JSON_T_DECREF
#endif
-static yajl_val get_json_output (guestfs_h *g, const char *filename);
+static json_t *get_json_output (guestfs_h *g, const char *filename);
static void set_child_rlimits (struct command *);
char *
guestfs_impl_disk_format (guestfs_h *g, const char *filename)
{
- size_t i, len;
- CLEANUP_YAJL_TREE_FREE yajl_val tree = get_json_output (g, filename);
+ CLEANUP_JSON_T_DECREF json_t *tree = get_json_output (g, filename);
+ json_t *node;
if (tree == NULL)
return NULL;
- if (! YAJL_IS_OBJECT (tree))
+ if (!json_is_object (tree))
goto bad_type;
- len = YAJL_GET_OBJECT(tree)->len;
- for (i = 0; i < len; ++i) {
- if (STREQ (YAJL_GET_OBJECT(tree)->keys[i], "format")) {
- const char *str;
- yajl_val node = YAJL_GET_OBJECT(tree)->values[i];
- if (YAJL_IS_NULL (node))
- goto bad_type;
- str = YAJL_GET_STRING (node);
- if (str == NULL)
- goto bad_type;
- return safe_strdup (g, str); /* caller frees */
- }
- }
+ node = json_object_get (tree, "format");
+ if (!json_is_string (node))
+ goto bad_type;
+
+ return safe_strndup (g, json_string_value (node),
+ json_string_length (node)); /* caller frees */
bad_type:
error (g, _("qemu-img info: JSON output did not contain ‘format’
key"));
@@ -94,30 +87,20 @@ guestfs_impl_disk_format (guestfs_h *g, const char
*filename)
int64_t
guestfs_impl_disk_virtual_size (guestfs_h *g, const char *filename)
{
- size_t i, len;
- CLEANUP_YAJL_TREE_FREE yajl_val tree = get_json_output (g, filename);
+ CLEANUP_JSON_T_DECREF json_t *tree = get_json_output (g, filename);
+ json_t *node;
if (tree == NULL)
return -1;
- if (! YAJL_IS_OBJECT (tree))
+ if (!json_is_object (tree))
goto bad_type;
- len = YAJL_GET_OBJECT(tree)->len;
- for (i = 0; i < len; ++i) {
- if (STREQ (YAJL_GET_OBJECT(tree)->keys[i], "virtual-size")) {
- yajl_val node = YAJL_GET_OBJECT(tree)->values[i];
- if (YAJL_IS_NULL (node))
- goto bad_type;
- if (! YAJL_IS_NUMBER (node))
- goto bad_type;
- if (! YAJL_IS_INTEGER (node)) {
- error (g, _("qemu-img info: ‘virtual-size’ is not representable as
a 64 bit integer"));
- return -1;
- }
- return YAJL_GET_INTEGER (node);
- }
- }
+ node = json_object_get (tree, "virtual-size");
+ if (!json_is_integer (node))
+ goto bad_type;
+
+ return json_integer_value (node);
bad_type:
error (g, _("qemu-img info: JSON output did not contain ‘virtual-size’
key"));
@@ -127,29 +110,25 @@ guestfs_impl_disk_virtual_size (guestfs_h *g, const char
*filename)
int
guestfs_impl_disk_has_backing_file (guestfs_h *g, const char *filename)
{
- size_t i, len;
- CLEANUP_YAJL_TREE_FREE yajl_val tree = get_json_output (g, filename);
+ CLEANUP_JSON_T_DECREF json_t *tree = get_json_output (g, filename);
+ json_t *node;
if (tree == NULL)
return -1;
- if (! YAJL_IS_OBJECT (tree))
+ if (!json_is_object (tree))
goto bad_type;
- len = YAJL_GET_OBJECT(tree)->len;
- for (i = 0; i < len; ++i) {
- if (STREQ (YAJL_GET_OBJECT(tree)->keys[i],
"backing-filename")) {
- yajl_val node = YAJL_GET_OBJECT(tree)->values[i];
- /* Work on the assumption that if this field is null, it means
- * no backing file, rather than being an error.
- */
- if (YAJL_IS_NULL (node))
- return 0;
- return 1;
- }
- }
+ node = json_object_get (tree, "backing-filename");
+ if (node == NULL)
+ return 0; /* no backing-filename key means no backing file */
- return 0; /* no backing-filename key means no backing file */
+ /* Work on the assumption that if this field is null, it means
+ * no backing file, rather than being an error.
+ */
+ if (json_is_null (node))
+ return 0;
+ return 1;
bad_type:
error (g, _("qemu-img info: JSON output was not an object"));
@@ -162,13 +141,13 @@ guestfs_impl_disk_has_backing_file (guestfs_h *g, const
char *filename)
static void parse_json (guestfs_h *g, void *treevp, const char *input, size_t
len);
#define PARSE_JSON_NO_OUTPUT ((void *) -1)
-static yajl_val
+static json_t *
get_json_output (guestfs_h *g, const char *filename)
{
CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g);
int fd, r;
char fdpath[64];
- yajl_val tree = NULL;
+ json_t *tree = NULL;
struct stat statbuf;
fd = open (filename, O_RDONLY /* NB: !O_CLOEXEC */);
@@ -217,16 +196,15 @@ get_json_output (guestfs_h *g, const char *filename)
return NULL;
}
- return tree; /* caller must call yajl_tree_free (tree) */
+ return tree; /* caller must call json_decref (tree) */
}
/* Parse the JSON document printed by qemu-img info --output json. */
static void
parse_json (guestfs_h *g, void *treevp, const char *input, size_t len)
{
- yajl_val *tree_ret = treevp;
- CLEANUP_FREE char *input_copy = NULL;
- char parse_error[256];
+ json_t **tree_ret = treevp;
+ json_error_t err;
assert (*tree_ret == NULL);
@@ -239,15 +217,12 @@ parse_json (guestfs_h *g, void *treevp, const char *input,
size_t len)
return;
}
- /* 'input' is not \0-terminated; we have to make it so. */
- input_copy = safe_strndup (g, input, len);
-
- debug (g, "%s: qemu-img info JSON output:\n%s\n", __func__,
input_copy);
+ debug (g, "%s: qemu-img info JSON output:\n%.*s\n", __func__, (int)
len, input);
- *tree_ret = yajl_tree_parse (input_copy, parse_error, sizeof parse_error);
+ *tree_ret = json_loadb (input, len, 0, &err);
if (*tree_ret == NULL) {
- if (strlen (parse_error) > 0)
- error (g, _("qemu-img info: JSON parse error: %s"),
parse_error);
+ if (strlen (err.text) > 0)
+ error (g, _("qemu-img info: JSON parse error: %s"), err.text);
else
error (g, _("qemu-img info: unknown JSON parse error"));
}
diff --git a/lib/qemu.c b/lib/qemu.c
index d27664733..b21897a29 100644
--- a/lib/qemu.c
+++ b/lib/qemu.c
@@ -37,7 +37,7 @@
#include <libxml/uri.h>
-#include <yajl/yajl_tree.h>
+#include <jansson.h>
#include "full-write.h"
#include "ignore-value.h"
@@ -57,7 +57,7 @@ struct qemu_data {
/* The following fields are derived from the fields above. */
struct version qemu_version; /* Parsed qemu version number. */
- yajl_val qmp_schema_tree; /* qmp_schema parsed into a JSON tree */
+ json_t *qmp_schema_tree; /* qmp_schema parsed into a JSON tree */
};
static char *cache_filename (guestfs_h *g, const char *cachedir, const struct
stat *, const char *suffix);
@@ -73,7 +73,7 @@ static int write_cache_qmp_schema (guestfs_h *g, const struct
qemu_data *data, c
static int read_cache_qemu_stat (guestfs_h *g, struct qemu_data *data, const
char *filename);
static int write_cache_qemu_stat (guestfs_h *g, const struct qemu_data *data,
const char *filename);
static void parse_qemu_version (guestfs_h *g, const char *, struct version
*qemu_version);
-static void parse_json (guestfs_h *g, const char *, yajl_val *);
+static void parse_json (guestfs_h *g, const char *, json_t **);
static void read_all (guestfs_h *g, void *retv, const char *buf, size_t len);
static int generic_read_cache (guestfs_h *g, const char *filename, char
**strp);
static int generic_write_cache (guestfs_h *g, const char *filename, const char
*str);
@@ -405,17 +405,17 @@ parse_qemu_version (guestfs_h *g, const char *qemu_help,
* is not possible.
*/
static void
-parse_json (guestfs_h *g, const char *json, yajl_val *treep)
+parse_json (guestfs_h *g, const char *json, json_t **treep)
{
- char parse_error[256] = "";
+ json_error_t err;
if (!json)
return;
- *treep = yajl_tree_parse (json, parse_error, sizeof parse_error);
+ *treep = json_loads (json, 0, &err);
if (*treep == NULL) {
- if (strlen (parse_error) > 0)
- debug (g, "QMP parse error: %s (ignored)", parse_error);
+ if (strlen (err.text) > 0)
+ debug (g, "QMP parse error: %s (ignored)", err.text);
else
debug (g, "QMP unknown parse error (ignored)");
}
@@ -580,17 +580,6 @@ guestfs_int_qemu_supports_device (guestfs_h *g,
return strstr (data->qemu_devices, device_name) != NULL;
}
-/* GCC can't work out that the YAJL_IS_<foo> test is sufficient to
- * ensure that YAJL_GET_<foo> later doesn't return NULL.
- */
-#pragma GCC diagnostic push
-#if defined(__GNUC__) && __GNUC__ >= 6 /* gcc >= 6 */
-#pragma GCC diagnostic ignored "-Wnull-dereference"
-#endif
-#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >=
4.8.0 */
-#pragma GCC diagnostic ignored "-Wnonnull"
-#endif
-
/**
* Test if the qemu binary uses mandatory file locking, added in
* QEMU >= 2.10 (but sometimes disabled).
@@ -599,11 +588,7 @@ int
guestfs_int_qemu_mandatory_locking (guestfs_h *g,
const struct qemu_data *data)
{
- const char *return_path[] = { "return", NULL };
- const char *meta_type_path[] = { "meta-type", NULL };
- const char *members_path[] = { "members", NULL };
- const char *name_path[] = { "name", NULL };
- yajl_val schema, v, meta_type, members, m, name;
+ json_t *schema, *v, *meta_type, *members, *m, *name;
size_t i, j;
/* If there's no QMP schema, fall back to checking the version. */
@@ -616,27 +601,24 @@ guestfs_int_qemu_mandatory_locking (guestfs_h *g,
* Extract the schema from the wrapper. Note the returned ‘schema’
* will be an array.
*/
- schema = yajl_tree_get (data->qmp_schema_tree, return_path, yajl_t_array);
- if (schema == NULL)
+ schema = json_object_get (data->qmp_schema_tree, "return");
+ if (!json_is_array (schema))
goto fallback;
- assert (YAJL_IS_ARRAY(schema));
/* Now look for any member of the array which has:
* { "meta-type": "object",
* "members": [ ... { "name": "locking", ...
} ... ] ... }
*/
- for (i = 0; i < YAJL_GET_ARRAY(schema)->len; ++i) {
- v = YAJL_GET_ARRAY(schema)->values[i];
- meta_type = yajl_tree_get (v, meta_type_path, yajl_t_string);
- if (meta_type && YAJL_IS_STRING (meta_type) &&
- STREQ (YAJL_GET_STRING (meta_type), "object")) {
- members = yajl_tree_get (v, members_path, yajl_t_array);
- if (members) {
- for (j = 0; j < YAJL_GET_ARRAY(members)->len; ++j) {
- m = YAJL_GET_ARRAY(members)->values[j];
- name = yajl_tree_get (m, name_path, yajl_t_string);
- if (name && YAJL_IS_STRING (name) &&
- STREQ (YAJL_GET_STRING (name), "locking"))
+ json_array_foreach (schema, i, v) {
+ meta_type = json_object_get (v, "meta-type");
+ if (json_is_string (meta_type) &&
+ STREQ (json_string_value (meta_type), "object")) {
+ members = json_object_get (v, "members");
+ if (json_is_array (members)) {
+ json_array_foreach (members, j, m) {
+ name = json_object_get (v, "name");
+ if (json_is_string (name) &&
+ STREQ (json_string_value (name), "locking"))
return 1;
}
}
@@ -646,8 +628,6 @@ guestfs_int_qemu_mandatory_locking (guestfs_h *g,
return 0;
}
-#pragma GCC diagnostic pop
-
/**
* Escape a qemu parameter.
*
@@ -995,7 +975,7 @@ guestfs_int_free_qemu_data (struct qemu_data *data)
free (data->qemu_help);
free (data->qemu_devices);
free (data->qmp_schema);
- yajl_tree_free (data->qmp_schema_tree);
+ json_decref (data->qmp_schema_tree);
free (data);
}
}
diff --git a/m4/guestfs-libraries.m4 b/m4/guestfs-libraries.m4
index 963f725bd..58a05ef1e 100644
--- a/m4/guestfs-libraries.m4
+++ b/m4/guestfs-libraries.m4
@@ -284,8 +284,8 @@ LIBS="$LIBS $LIBXML2_LIBS"
AC_CHECK_FUNCS([xmlBufferDetach])
LIBS="$old_LIBS"
-dnl Check for yajl JSON library (required).
-PKG_CHECK_MODULES([YAJL], [yajl >= 2.0.4])
+dnl Check for Jansson JSON library (required).
+PKG_CHECK_MODULES([JANSSON], [jansson])
dnl Check for C++ (optional, we just use this to test the header works).
AC_PROG_CXX
--
2.13.6
Martin Kletzander
2017-Nov-23 14:00 UTC
Re: [Libguestfs] [PATCH 1/1] Switch from YAJL to Jansson
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.
On 11/23/2017 07:23 AM, 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.Which RFC? RFC 4627 stated that only objects or arrays could occur at the top level. But it was superseded by: RFC 7159, which states that ANY json value can be the top level of the grammar. So maybe it's better to set the flags and always allow all objects. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2017-Nov-23 18:51 UTC
Re: [Libguestfs] [PATCH 0/1] RFC: switch from YAJL to Jansson
On Thu, Nov 23, 2017 at 02:23:51PM +0100, Pino Toscano wrote:> What do you think?A quick reply is: yes, we should do this. But I think we might want to wait until after 1.38 is released (just a few more weeks), because it is quite a big change so close to the stable release. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top