From 2630980f49088a06c32cebdc866e4f518106af29 Mon Sep 17 00:00:00 2001 From: Petri Lehtinen Date: Wed, 12 May 2010 15:41:09 +0300 Subject: [PATCH 1/3] Zero the visited flag after encoding an empty array or object Encoding an empty array or object worked, but encoding it again (possibly after adding some items) failed, because the visited flag (used for detecting circular references) wasn't zeroed. --- src/dump.c | 8 +++++-- test/.gitignore | 1 + test/suites/api/Makefile.am | 2 ++ test/suites/api/test_dump.c | 46 +++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 test/suites/api/test_dump.c diff --git a/src/dump.c b/src/dump.c index bc06dfd..a36da03 100644 --- a/src/dump.c +++ b/src/dump.c @@ -231,8 +231,10 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, if(dump("[", 1, data)) return -1; - if(n == 0) + if(n == 0) { + array->visited = 0; return dump("]", 1, data); + } if(dump_indent(flags, depth + 1, 0, dump, data)) return -1; @@ -284,8 +286,10 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, if(dump("{", 1, data)) return -1; - if(!iter) + if(!iter) { + object->visited = 0; return dump("}", 1, data); + } if(dump_indent(flags, depth + 1, 0, dump, data)) return -1; diff --git a/test/.gitignore b/test/.gitignore index a960c50..44ff748 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -3,6 +3,7 @@ bin/json_process suites/api/test_array suites/api/test_equal suites/api/test_copy +suites/api/test_dump suites/api/test_load suites/api/test_number suites/api/test_object diff --git a/test/suites/api/Makefile.am b/test/suites/api/Makefile.am index 64523c0..7125792 100644 --- a/test/suites/api/Makefile.am +++ b/test/suites/api/Makefile.am @@ -4,6 +4,7 @@ check_PROGRAMS = \ test_array \ test_equal \ test_copy \ + test_dump \ test_load \ test_simple \ test_number \ @@ -11,6 +12,7 @@ check_PROGRAMS = \ test_array_SOURCES = test_array.c util.h test_copy_SOURCES = test_copy.c util.h +test_dump_SOURCES = test_dump.c util.h test_load_SOURCES = test_load.c util.h test_simple_SOURCES = test_simple.c util.h test_number_SOURCES = test_number.c util.h diff --git a/test/suites/api/test_dump.c b/test/suites/api/test_dump.c new file mode 100644 index 0000000..c471159 --- /dev/null +++ b/test/suites/api/test_dump.c @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2009, 2010 Petri Lehtinen + * + * Jansson is free software; you can redistribute it and/or modify + * it under the terms of the MIT license. See LICENSE for details. + */ + +#include +#include +#include "util.h" + +int main() +{ + json_t *json; + char *result; + + /* Encode an empty object/array, add an item, encode again */ + + json = json_object(); + result = json_dumps(json, 0); + if(!result || strcmp(result, "{}")) + fail("json_dumps failed"); + + json_object_set_new(json, "foo", json_integer(5)); + result = json_dumps(json, 0); + if(!result || strcmp(result, "{\"foo\": 5}")) + fail("json_dumps failed"); + + json_decref(json); + + json = json_array(); + result = json_dumps(json, 0); + if(!result || strcmp(result, "[]")) + fail("json_dumps failed"); + free(result); + + json_array_append_new(json, json_integer(5)); + result = json_dumps(json, 0); + if(!result || strcmp(result, "[5]")) + fail("json_dumps failed"); + free(result); + + json_decref(json); + + return 0; +} From 453e4c0aa2fbca95676fa966cc5602f5daf0a1f2 Mon Sep 17 00:00:00 2001 From: Petri Lehtinen Date: Fri, 14 May 2010 08:47:24 +0300 Subject: [PATCH 2/3] Zero the visited flag after an encoding error When encoding an array or object ends in an error, the visited flag wasn't zeroed, causing subsequent encoding attempts to fail. This patch fixes the problem by always zeroing the visited flag. --- src/dump.c | 40 ++++++++++++++++++++-------------- test/suites/api/test_dump.c | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/src/dump.c b/src/dump.c index a36da03..870f30e 100644 --- a/src/dump.c +++ b/src/dump.c @@ -224,40 +224,44 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, /* detect circular references */ array = json_to_array(json); if(array->visited) - return -1; + goto array_error; array->visited = 1; n = json_array_size(json); if(dump("[", 1, data)) - return -1; + goto array_error; if(n == 0) { array->visited = 0; return dump("]", 1, data); } if(dump_indent(flags, depth + 1, 0, dump, data)) - return -1; + goto array_error; for(i = 0; i < n; ++i) { if(do_dump(json_array_get(json, i), flags, depth + 1, dump, data)) - return -1; + goto array_error; if(i < n - 1) { if(dump(",", 1, data) || dump_indent(flags, depth + 1, 1, dump, data)) - return -1; + goto array_error; } else { if(dump_indent(flags, depth, 0, dump, data)) - return -1; + goto array_error; } } array->visited = 0; return dump("]", 1, data); + + array_error: + array->visited = 0; + return -1; } case JSON_OBJECT: @@ -279,19 +283,19 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, /* detect circular references */ object = json_to_object(json); if(object->visited) - return -1; + goto object_error; object->visited = 1; iter = json_object_iter((json_t *)json); if(dump("{", 1, data)) - return -1; + goto object_error; if(!iter) { object->visited = 0; return dump("}", 1, data); } if(dump_indent(flags, depth + 1, 0, dump, data)) - return -1; + goto object_error; if(flags & JSON_SORT_KEYS) { @@ -304,7 +308,7 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, size = json_object_size(json); keys = malloc(size * sizeof(const char *)); if(!keys) - return -1; + goto object_error; i = 0; while(iter) @@ -331,7 +335,7 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, do_dump(value, flags, depth + 1, dump, data)) { free(keys); - return -1; + goto object_error; } if(i < size - 1) @@ -340,7 +344,7 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, dump_indent(flags, depth + 1, 1, dump, data)) { free(keys); - return -1; + goto object_error; } } else @@ -348,7 +352,7 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, if(dump_indent(flags, depth, 0, dump, data)) { free(keys); - return -1; + goto object_error; } } } @@ -367,18 +371,18 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, if(dump(separator, separator_length, data) || do_dump(json_object_iter_value(iter), flags, depth + 1, dump, data)) - return -1; + goto object_error; if(next) { if(dump(",", 1, data) || dump_indent(flags, depth + 1, 1, dump, data)) - return -1; + goto object_error; } else { if(dump_indent(flags, depth, 0, dump, data)) - return -1; + goto object_error; } iter = next; @@ -387,6 +391,10 @@ static int do_dump(const json_t *json, unsigned long flags, int depth, object->visited = 0; return dump("}", 1, data); + + object_error: + object->visited = 0; + return -1; } default: diff --git a/test/suites/api/test_dump.c b/test/suites/api/test_dump.c index c471159..97eb03e 100644 --- a/test/suites/api/test_dump.c +++ b/test/suites/api/test_dump.c @@ -42,5 +42,48 @@ int main() json_decref(json); + /* Construct a JSON object/array with a circular reference: + + object: {"a": {"b": {"c": }}} + array: [[[]]] + + Encode it, remove the circular reference and encode again. + */ + json = json_object(); + json_object_set_new(json, "a", json_object()); + json_object_set_new(json_object_get(json, "a"), "b", json_object()); + json_object_set(json_object_get(json_object_get(json, "a"), "b"), "c", + json_object_get(json, "a")); + + if(json_dumps(json, 0)) + fail("json_dumps encoded a circular reference!"); + + json_object_del(json_object_get(json_object_get(json, "a"), "b"), "c"); + + result = json_dumps(json, 0); + if(!result || strcmp(result, "{\"a\": {\"b\": {}}}")) + fail("json_dumps failed!"); + free(result); + + json_decref(json); + + json = json_array(); + json_array_append_new(json, json_array()); + json_array_append_new(json_array_get(json, 0), json_array()); + json_array_append(json_array_get(json_array_get(json, 0), 0), + json_array_get(json, 0)); + + if(json_dumps(json, 0)) + fail("json_dumps encoded a circular reference!"); + + json_array_remove(json_array_get(json_array_get(json, 0), 0), 0); + + result = json_dumps(json, 0); + if(!result || strcmp(result, "[[[]]]")) + fail("json_dumps failed!"); + free(result); + + json_decref(json); + return 0; } From 978a47e2c52cca0999bbc8eb6d35674d4931ed5c Mon Sep 17 00:00:00 2001 From: Petri Lehtinen Date: Thu, 20 May 2010 18:45:06 +0300 Subject: [PATCH 3/3] Clarify the documentation on reference stealing Provide an example usage pattern for reference stealing functions. This way the user (hopely) sees more clearly how the reference stealing functions are meant to be used. --- doc/apiref.rst | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/doc/apiref.rst b/doc/apiref.rst index a2a0794..0db662d 100644 --- a/doc/apiref.rst +++ b/doc/apiref.rst @@ -154,9 +154,31 @@ Normally, all functions accepting a JSON value as an argument will manage the reference, i.e. increase and decrease the reference count as needed. However, some functions **steal** the reference, i.e. they have the same result as if the user called :cfunc:`json_decref()` on -the argument right after calling the function. These are usually -convenience functions for adding new references to containers and not -to worry about the reference count. +the argument right after calling the function. These functions are +suffixed with ``_new`` or have ``_new_`` somewhere in their name. + +For example, the following code creates a new JSON array and appends +an integer to it:: + + json_t *array, *integer; + + array = json_array(); + integer = json_integer(42); + + json_array_append(array, integer); + json_decref(integer); + +Note how the caller has to release the reference to the integer value +by calling :cfunc:`json_decref()`. By using a reference stealing +function :cfunc:`json_array_append_new()` instead of +:cfunc:`json_array_append()`, the code becomes much simpler:: + + json_t *array = json_array(); + json_array_append_new(array, json_integer(42)); + +In this case, the user doesn't have to explicitly release the +reference to the integer value, as :cfunc:`json_array_append_new()` +steals the reference when appending the value to the array. In the following sections it is clearly documented whether a function will return a new or borrowed reference or steal a reference to its