fix json_object_put() to return 1 for freed scalars and empty containers

The iterative rewrite in 17328a67 made json_object_put() return 0 for freed scalars, strings and empty containers, contradicting the documented contract in json_object.h. Distinguish "freed" from "still referenced" in _json_object_put_maybe_free() and add regression tests.

diff --git a/json_object.c b/json_object.c
index ca4c089f69d88f0802ef34040a27d7d9bb5db72e..13d3826d97c792705fc3686ee97f249b8c5ba88b 100644 (file)
--- a/json_object.c
+++ b/json_object.c
@@ -273,13 +273,31 @@ struct json_object *json_object_get(struct json_object *jso)
 }
 
 
+/**
+  * Return values for _json_object_put_maybe_free().
+  * json_object_put_still_refd and json_object_put_freed match the documented
+  * return values of json_object_put(), so they can be returned directly.
+  */
+enum json_object_put_result
+{
+       json_object_put_still_refd = 0, /* refcount decremented, object not freed */
+       json_object_put_freed = 1,      /* refcount reached zero, memory released */
+       json_object_put_container = 2   /* refcount reached zero, but the object is a
+                                          non-empty container: the caller must free
+                                          the contents, then the object itself */
+};
+
 /**
   * Internal json_object_put function
-  * Returns 0 if we're done "freeing" the object, either because its memory
-  * was actually released, or we just needed to decrement the refcount.
-  * Returns 1 when the object is a non-empty container that still needs to be handled.
+  * Returns json_object_put_still_refd if a reference remains, and the object
+  * was not freed.
+  * Returns json_object_put_freed if the object's memory was released, either
+  * because it holds no other objects, or because free_containers was set.
+  * Returns json_object_put_container when the refcount reached zero but the
+  * object is a non-empty container; the caller must free the contents, then
+  * call this function again with free_containers set to free the object itself.
   */
-static inline int _json_object_put_maybe_free(struct json_object *jso, int free_containers)
+static inline enum json_object_put_result _json_object_put_maybe_free(struct json_object *jso, int free_containers)
 {
        /* Avoid invalid free and crash explicitly instead of (silently)
         * segfaulting.
@@ -298,7 +316,7 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
        if (--jso->_ref_count > 0)
 #endif
        {
-               return 0;  // All done, caller doesn't need to do anything else
+               return json_object_put_still_refd;
        }
 
        if (jso->_user_delete)
@@ -315,15 +333,15 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
                        json_object_object_delete(jso);
                        break;
                }
-               return 1;
-       case json_type_array: 
+               return json_object_put_container;
+       case json_type_array:
                // container objects are handled by the caller
                if (free_containers || array_list_length(JC_ARRAY(jso)->c_array) == 0)
                {
                        json_object_array_delete(jso);
                        break;
                }
-               return 1;
+               return json_object_put_container;
        case json_type_string:
                json_object_string_delete(jso);
                break;
@@ -331,16 +349,19 @@ static inline int _json_object_put_maybe_free(struct json_object *jso, int free_
                json_object_generic_delete(jso);
                break;
        }
-       return 0;  // All done, caller doesn't need to do anything else
+       return json_object_put_freed;
 }
 
 int json_object_put(struct json_object *jso)
 {
+       enum json_object_put_result rc;
+
        if (!jso)
                return 0;
 
-       if (_json_object_put_maybe_free(jso, 0) == 0)
-               return 0;
+       rc = _json_object_put_maybe_free(jso, 0);
+       if (rc != json_object_put_container)
+               return (int)rc;
        // else, it's a non-empty container object, handle it below
 
        // Note: jso is now a "zombie" object, _ref_count == 0 but memory not yet released
@@ -399,7 +420,7 @@ int json_object_put(struct json_object *jso)
                        }
 
                        // Now, handle actually freeing the json_object in that slot
-                       if (!child || _json_object_put_maybe_free(child, 0) == 0)
+                       if (!child || _json_object_put_maybe_free(child, 0) != json_object_put_container)
                        {
                                // child is either freed, or still referenced somewhere else
                                // leave it as-is and handle the previous slot
@@ -441,13 +462,13 @@ int json_object_put(struct json_object *jso)
                        // so we need to actually free it now
                        // Be sure to grab _delete_parent *before* freeing jso.
                        json_object *parent = jso->_delete_parent;
-                       int rc;
+                       enum json_object_put_result rc;
                        assert(jso->_ref_count == 0);
                        jso->_ref_count++;   // We're the exclusive owner of jso, non-atomic add is ok.
                        // Note: the call must not be inside assert(), or it gets
                        // compiled out when NDEBUG is defined and the memory leaks.
                        rc = _json_object_put_maybe_free(jso, 1);
-                       assert(rc == 0);
+                       assert(rc == json_object_put_freed);
                        (void)rc;
                        jso = parent;
                        // iteration will be reset at the top of the loop

diff --git a/tests/test_deep_nesting.c b/tests/test_deep_nesting.c
index 9e251750c480bc0d5caaaca0faffee4f3145ccce..4bf24578daa127209d5d8a3e7c3ea5d8a361334d 100644 (file)
--- a/tests/test_deep_nesting.c
+++ b/tests/test_deep_nesting.c
@@ -81,12 +81,36 @@ static void test_nesting_with_user_delete(void)
                1, malloc(8192)
        };
 
-       jso = json_object_new_object();
+       jso = json_object_new_object();
        json_object_set_userdata(jso, &userdata_val, user_delete_test);
        json_object_object_add(jso, "somekey", json_object_new_string("foo"));
        json_object_put(jso);
 }
 
+/*
+ * Check that json_object_put() returns 1 whenever the object is freed,
+ * including for scalars and empty containers, and 0 only when a
+ * reference remains.
+ */
+static void test_put_return_values(void)
+{
+       json_object *jso;
+
+       printf("put(empty object) returned %d\n", json_object_put(json_object_new_object()));
+       printf("put(empty array) returned %d\n", json_object_put(json_object_new_array()));
+       printf("put(string) returned %d\n", json_object_put(json_object_new_string("foo")));
+       printf("put(int) returned %d\n", json_object_put(json_object_new_int(42)));
+
+       jso = json_object_new_object();
+       json_object_object_add(jso, "somekey", json_object_new_string("foo"));
+       printf("put(non-empty object) returned %d\n", json_object_put(jso));
+
+       jso = json_object_new_object();
+       json_object_get(jso);
+       printf("put(still-referenced object) returned %d\n", json_object_put(jso));
+       printf("put(last reference) returned %d\n", json_object_put(jso));
+}
+
 int main(int argc, char **argv)
 {
        char *str;      
@@ -110,5 +134,7 @@ int main(int argc, char **argv)
 
        test_nesting_with_user_delete();
 
+       test_put_return_values();
+
        return EXIT_SUCCESS;
 }

diff --git a/tests/test_deep_nesting.expected b/tests/test_deep_nesting.expected
index b9c1ab98480f8da2a0ff417db5d291adb9d4ecee..2449ede3265c462ae4e175564b808dd48db9e83a 100644 (file)
--- a/tests/test_deep_nesting.expected
+++ b/tests/test_deep_nesting.expected
@@ -1,3 +1,10 @@
 Parsed depth 100000 string to json_object: yes
 Freed json_object
 in user_delete, userdata_val=1
+put(empty object) returned 1
+put(empty array) returned 1
+put(string) returned 1
+put(int) returned 1
+put(non-empty object) returned 1
+put(still-referenced object) returned 0
+put(last reference) returned 1