json: Fix deep copy of objects and arrays.

When reference counting for json objects was introduced the old json_clone() function became json_deep_clone(), but it still calls shallow json_clone() while cloning objects and arrays not really producing a deep copy. Fixing that by making other functions to perform a deep copy as well. There are no users for this functionality inside OVS right now, but OVS exports this functionality externally. 'ovstest test-json' extended to test both versions of a clone on provided inputs. Fixes: 9854d47 ("json: Use reference counting in JSON objects") Acked-by: Dumitru Ceara <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
nevalosa · Oct 11, 2022 · 0b21e23 · 0b21e23
1 parent b5d9722
commit 0b21e23
Show file tree

Hide file tree

Showing 2 changed files with 128 additions and 12 deletions.
diff --git a/lib/json.c b/lib/json.c
@@ -420,19 +420,19 @@ json_destroy_array(struct json_array *array)
     free(array->elems);
 }
 
-static struct json *json_clone_object(const struct shash *object);
-static struct json *json_clone_array(const struct json_array *array);
+static struct json *json_deep_clone_object(const struct shash *object);
+static struct json *json_deep_clone_array(const struct json_array *array);
 
 /* Returns a deep copy of 'json'. */
 struct json *
 json_deep_clone(const struct json *json)
 {
     switch (json->type) {
     case JSON_OBJECT:
-        return json_clone_object(json->object);
+        return json_deep_clone_object(json->object);
 
     case JSON_ARRAY:
-        return json_clone_array(&json->array);
+        return json_deep_clone_array(&json->array);
 
     case JSON_STRING:
         return json_string_create(json->string);
@@ -464,28 +464,28 @@ json_nullable_clone(const struct json *json)
 }
 
 static struct json *
-json_clone_object(const struct shash *object)
+json_deep_clone_object(const struct shash *object)
 {
     struct shash_node *node;
     struct json *json;
 
     json = json_object_create();
     SHASH_FOR_EACH (node, object) {
         struct json *value = node->data;
-        json_object_put(json, node->name, json_clone(value));
+        json_object_put(json, node->name, json_deep_clone(value));
     }
     return json;
 }
 
 static struct json *
-json_clone_array(const struct json_array *array)
+json_deep_clone_array(const struct json_array *array)
 {
     struct json **elems;
     size_t i;
 
     elems = xmalloc(array->n * sizeof *elems);
     for (i = 0; i < array->n; i++) {
-        elems[i] = json_clone(array->elems[i]);
+        elems[i] = json_deep_clone(array->elems[i]);
     }
     return json_array_create(elems, array->n);
 }

diff --git a/tests/test-json.c b/tests/test-json.c
@@ -34,8 +34,123 @@ static int pretty = 0;
  * instead of exactly one object or array. */
 static int multiple = 0;
 
+static void test_json_equal(const struct json *a, const struct json *b,
+                            bool allow_the_same);
+
+static void
+test_json_equal_object(const struct shash *a, const struct shash *b,
+                       bool allow_the_same)
+{
+    struct shash_node *a_node;
+
+    ovs_assert(allow_the_same || a != b);
+
+    if (a == b) {
+        return;
+    }
+
+    ovs_assert(shash_count(a) == shash_count(b));
+
+    SHASH_FOR_EACH (a_node, a) {
+        struct shash_node *b_node = shash_find(b, a_node->name);
+
+        ovs_assert(b_node);
+        test_json_equal(a_node->data, b_node->data, allow_the_same);
+    }
+}
+
+static void
+test_json_equal_array(const struct json_array *a, const struct json_array *b,
+                      bool allow_the_same)
+{
+    ovs_assert(allow_the_same || a != b);
+
+    if (a == b) {
+        return;
+    }
+
+    ovs_assert(a->n == b->n);
+
+    for (size_t i = 0; i < a->n; i++) {
+        test_json_equal(a->elems[i], b->elems[i], allow_the_same);
+    }
+}
+
+static void
+test_json_equal(const struct json *a, const struct json *b,
+                bool allow_the_same)
+{
+    ovs_assert(allow_the_same || a != b);
+    ovs_assert(a && b);
+
+    if (a == b) {
+        ovs_assert(a->count > 1);
+        return;
+    }
+
+    ovs_assert(a->type == b->type);
+
+    switch (a->type) {
+    case JSON_OBJECT:
+        test_json_equal_object(a->object, b->object, allow_the_same);
+        return;
+
+    case JSON_ARRAY:
+        test_json_equal_array(&a->array, &b->array, allow_the_same);
+        return;
+
+    case JSON_STRING:
+    case JSON_SERIALIZED_OBJECT:
+        ovs_assert(a->string != b->string);
+        ovs_assert(!strcmp(a->string, b->string));
+        return;
+
+    case JSON_NULL:
+    case JSON_FALSE:
+    case JSON_TRUE:
+        return;
+
+    case JSON_INTEGER:
+        ovs_assert(a->integer == b->integer);
+        return;
+
+    case JSON_REAL:
+        ovs_assert(a->real == b->real);
+        return;
+
+    case JSON_N_TYPES:
+    default:
+        OVS_NOT_REACHED();
+    }
+}
+
+static void
+test_json_clone(struct json *json)
+{
+    struct json *copy, *deep_copy;
+
+    copy = json_clone(json);
+
+    ovs_assert(json_equal(json, copy));
+    test_json_equal(json, copy, true);
+    ovs_assert(json->count == 2);
+
+    json_destroy(copy);
+    ovs_assert(json->count == 1);
+
+    deep_copy = json_deep_clone(json);
+
+    ovs_assert(json_equal(json, deep_copy));
+    test_json_equal(json, deep_copy, false);
+    ovs_assert(json->count == 1);
+    ovs_assert(deep_copy->count == 1);
+
+    json_destroy(deep_copy);
+    ovs_assert(json->count == 1);
+}
+
 static bool
-print_and_free_json(struct json *json)
+print_test_and_free_json(struct json *json)
 {
     bool ok;
     if (json->type == JSON_STRING) {
@@ -47,6 +162,7 @@ print_and_free_json(struct json *json)
         free(s);
         ok = true;
     }
+    test_json_clone(json);
     json_destroy(json);
     return ok;
 }
@@ -89,15 +205,15 @@ parse_multiple(FILE *stream)
 
             used += json_parser_feed(parser, &buffer[used], n - used);
             if (used < n) {
-                if (!print_and_free_json(json_parser_finish(parser))) {
+                if (!print_test_and_free_json(json_parser_finish(parser))) {
                     ok = false;
                 }
                 parser = NULL;
             }
         }
     }
     if (parser) {
-        if (!print_and_free_json(json_parser_finish(parser))) {
+        if (!print_test_and_free_json(json_parser_finish(parser))) {
             ok = false;
         }
     }
@@ -150,7 +266,7 @@ test_json_main(int argc, char *argv[])
     if (multiple) {
         ok = parse_multiple(stream);
     } else {
-        ok = print_and_free_json(json_from_stream(stream));
+        ok = print_test_and_free_json(json_from_stream(stream));
     }
 
     fclose(stream);