From 6b35f1c5bc347377ea32881914a0fe5d60e730e6 Mon Sep 17 00:00:00 2001 From: Alanscut Date: Tue, 24 Mar 2020 22:28:15 +0800 Subject: [PATCH 1/4] add new function of setValuestringToObject --- cJSON.c | 25 +++++++++++++++++++++++++ cJSON.h | 2 ++ 2 files changed, 27 insertions(+) diff --git a/cJSON.c b/cJSON.c index b0e744e..ff8d2b9 100644 --- a/cJSON.c +++ b/cJSON.c @@ -368,6 +368,31 @@ CJSON_PUBLIC(double) cJSON_SetNumberHelper(cJSON *object, double number) return object->valuedouble = number; } +CJSON_PUBLIC(char*) cJSON_SetValuestringToObject(cJSON *object, const char *valuestring) +{ + size_t length = 0; + char *copy = NULL; + /* if object->valuestring is NULL, it should not set valuestring */ + if (object->valuestring == NULL) + { + return NULL; + } + length = strlen(valuestring) + sizeof(""); + copy = (char*) cJSON_malloc(length); + if (copy == NULL) + { + return NULL; + } + memcpy(copy, valuestring, length); + if (!(object->type & cJSON_IsReference) && (object->valuestring != NULL)) + { + cJSON_free(object->valuestring); + } + object->valuestring = copy; + + return copy; +} + typedef struct { unsigned char *buffer; diff --git a/cJSON.h b/cJSON.h index 2c53562..1455ece 100644 --- a/cJSON.h +++ b/cJSON.h @@ -278,6 +278,8 @@ CJSON_PUBLIC(cJSON*) cJSON_AddArrayToObject(cJSON * const object, const char * c /* helper for the cJSON_SetNumberValue macro */ CJSON_PUBLIC(double) cJSON_SetNumberHelper(cJSON *object, double number); #define cJSON_SetNumberValue(object, number) ((object != NULL) ? cJSON_SetNumberHelper(object, (double)number) : (number)) +/* Only takes effect when type of object is JSON_Object */ +CJSON_PUBLIC(char*) cJSON_SetValuestringToObject(cJSON *object, const char *valuestring); /* Macro for iterating over an array or object */ #define cJSON_ArrayForEach(element, array) for(element = (array != NULL) ? (array)->child : NULL; element != NULL; element = element->next) From 4790c3c8f52cc08ad584e148c4e3d93d9d063f0c Mon Sep 17 00:00:00 2001 From: Alanscut Date: Tue, 24 Mar 2020 22:28:27 +0800 Subject: [PATCH 2/4] add testcase for cJSON_SetValuestringToObject --- tests/misc_tests.c | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/tests/misc_tests.c b/tests/misc_tests.c index 20a5ddd..ba494f5 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -550,19 +550,20 @@ static void cjson_add_item_to_object_should_not_use_after_free_when_string_is_al cJSON_Delete(object); } -static void cjson_delete_item_from_array_should_not_broken_list_structure(void) { +static void cjson_delete_item_from_array_should_not_broken_list_structure(void) +{ const char expected_json1[] = "{\"rd\":[{\"a\":\"123\"}]}"; const char expected_json2[] = "{\"rd\":[{\"a\":\"123\"},{\"b\":\"456\"}]}"; const char expected_json3[] = "{\"rd\":[{\"b\":\"456\"}]}"; - char* str1 = NULL; - char* str2 = NULL; - char* str3 = NULL; + char *str1 = NULL; + char *str2 = NULL; + char *str3 = NULL; - cJSON* root = cJSON_Parse("{}"); + cJSON *root = cJSON_Parse("{}"); - cJSON* array = cJSON_AddArrayToObject(root, "rd"); - cJSON* item1 = cJSON_Parse("{\"a\":\"123\"}"); - cJSON* item2 = cJSON_Parse("{\"b\":\"456\"}"); + cJSON *array = cJSON_AddArrayToObject(root, "rd"); + cJSON *item1 = cJSON_Parse("{\"a\":\"123\"}"); + cJSON *item2 = cJSON_Parse("{\"b\":\"456\"}"); cJSON_AddItemToArray(array, item1); str1 = cJSON_PrintUnformatted(root); @@ -583,6 +584,31 @@ static void cjson_delete_item_from_array_should_not_broken_list_structure(void) cJSON_Delete(root); } +static void cjson_set_valuestring_to_object_should_not_leak_memory(void) +{ + cJSON *root = cJSON_Parse("{}"); + cJSON *item1 = cJSON_CreateString("valuestring could be changed safely"); + cJSON *item2 = cJSON_CreateStringReference("reference item should be freed by yourself"); + const char *newValuestring = "new valuestring which much longer than previous"; + char *returnValue = NULL; + + cJSON_AddItemToObject(root, "one", item1); + cJSON_AddItemToObject(root, "two", item2); + + /* we needn't to free the original valuestring manually */ + returnValue = cJSON_SetValuestringToObject(cJSON_GetObjectItem(root, "one"), newValuestring); + TEST_ASSERT_NOT_NULL(returnValue); + TEST_ASSERT_EQUAL_STRING(newValuestring, cJSON_GetObjectItem(root, "one")->valuestring); + + returnValue = cJSON_SetValuestringToObject(cJSON_GetObjectItem(root, "two"), newValuestring); + TEST_ASSERT_NOT_NULL(returnValue); + TEST_ASSERT_EQUAL_STRING(newValuestring, cJSON_GetObjectItem(root, "two")->valuestring); + /* we must free the memory manually when the item's type is cJSON_IsReference */ + cJSON_free(item2->valuestring); + + cJSON_Delete(root); +} + int CJSON_CDECL main(void) { UNITY_BEGIN(); @@ -609,6 +635,7 @@ int CJSON_CDECL main(void) RUN_TEST(cjson_create_array_reference_should_create_an_array_reference); RUN_TEST(cjson_add_item_to_object_should_not_use_after_free_when_string_is_aliased); RUN_TEST(cjson_delete_item_from_array_should_not_broken_list_structure); + RUN_TEST(cjson_set_valuestring_to_object_should_not_leak_memory); return UNITY_END(); } From 34e102d0dc91d274b66b5faa434140ac90a92598 Mon Sep 17 00:00:00 2001 From: Alanscut Date: Tue, 24 Mar 2020 22:28:35 +0800 Subject: [PATCH 3/4] update README --- README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 55aba58..f074b63 100644 --- a/README.md +++ b/README.md @@ -197,21 +197,22 @@ The type can be one of the following: * `cJSON_NULL` (check with `cJSON_IsNull`): Represents a `null` value. * `cJSON_Number` (check with `cJSON_IsNumber`): Represents a number value. The value is stored as a double in `valuedouble` and also in `valueint`. If the number is outside of the range of an integer, `INT_MAX` or `INT_MIN` are used for `valueint`. * `cJSON_String` (check with `cJSON_IsString`): Represents a string value. It is stored in the form of a zero terminated string in `valuestring`. -* `cJSON_Array` (check with `cJSON_IsArray`): Represent an array value. This is implemented by pointing `child` to a linked list of `cJSON` items that represent the values in the array. The elements are linked together using `next` and `prev`, where the first element has `prev == NULL` and the last element `next == NULL`. +* `cJSON_Array` (check with `cJSON_IsArray`): Represent an array value. This is implemented by pointing `child` to a linked list of `cJSON` items that represent the values in the array. The elements are linked together using `next` and `prev`, where the first element has `prev.next == NULL` and the last element `next == NULL`. * `cJSON_Object` (check with `cJSON_IsObject`): Represents an object value. Objects are stored same way as an array, the only difference is that the items in the object store their keys in `string`. * `cJSON_Raw` (check with `cJSON_IsRaw`): Represents any kind of JSON that is stored as a zero terminated array of characters in `valuestring`. This can be used, for example, to avoid printing the same static JSON over and over again to save performance. cJSON will never create this type when parsing. Also note that cJSON doesn't check if it is valid JSON. Additionally there are the following two flags: -* `cJSON_IsReference`: Specifies that the item that `child` points to and/or `valuestring` is not owned by this item, it is only a reference. So `cJSON_Delete` and other functions will only deallocate this item, not its children/valuestring. +* `cJSON_IsReference`: Specifies that the item that `child` points to and/or `valuestring` is not owned by this item, it is only a reference. So `cJSON_Delete` and other functions will only deallocate this item, not its `child`/`valuestring`. * `cJSON_StringIsConst`: This means that `string` points to a constant string. This means that `cJSON_Delete` and other functions will not try to deallocate `string`. ### Working with the data structure For every value type there is a `cJSON_Create...` function that can be used to create an item of that type. All of these will allocate a `cJSON` struct that can later be deleted with `cJSON_Delete`. -Note that you have to delete them at some point, otherwise you will get a memory leak. -**Important**: If you have added an item to an array or an object already, you **mustn't** delete it with `cJSON_Delete`. Adding it to an array or object transfers its ownership so that when that array or object is deleted, it gets deleted as well. +Note that you have to delete them at some point, otherwise you will get a memory leak. +**Important**: If you have added an item to an array or an object already, you **mustn't** delete it with `cJSON_Delete`. Adding it to an array or object transfers its ownership so that when that array or object is deleted, +it gets deleted as well. You also could use `cJSON_SetValuestringToObject` to change a `cJSON_Object`'s `valuestring`, and you needn't to free the previous `valuestring` manually. #### Basic types From 31c7880fab5722a6fef47589c5064f7445d39f1d Mon Sep 17 00:00:00 2001 From: Alanscut Date: Wed, 25 Mar 2020 15:38:54 +0800 Subject: [PATCH 4/4] update cJSON_SetValuestring and testcase --- README.md | 2 +- cJSON.c | 18 ++++++++++-------- cJSON.h | 4 ++-- tests/misc_tests.c | 36 +++++++++++++++++++++++------------- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index f074b63..5480b8e 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ For every value type there is a `cJSON_Create...` function that can be used to c All of these will allocate a `cJSON` struct that can later be deleted with `cJSON_Delete`. Note that you have to delete them at some point, otherwise you will get a memory leak. **Important**: If you have added an item to an array or an object already, you **mustn't** delete it with `cJSON_Delete`. Adding it to an array or object transfers its ownership so that when that array or object is deleted, -it gets deleted as well. You also could use `cJSON_SetValuestringToObject` to change a `cJSON_Object`'s `valuestring`, and you needn't to free the previous `valuestring` manually. +it gets deleted as well. You also could use `cJSON_SetValuestring` to change a `cJSON_String`'s `valuestring`, and you needn't to free the previous `valuestring` manually. #### Basic types diff --git a/cJSON.c b/cJSON.c index ff8d2b9..2433de3 100644 --- a/cJSON.c +++ b/cJSON.c @@ -368,23 +368,25 @@ CJSON_PUBLIC(double) cJSON_SetNumberHelper(cJSON *object, double number) return object->valuedouble = number; } -CJSON_PUBLIC(char*) cJSON_SetValuestringToObject(cJSON *object, const char *valuestring) +CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring) { - size_t length = 0; char *copy = NULL; - /* if object->valuestring is NULL, it should not set valuestring */ - if (object->valuestring == NULL) + /* if object's type is not cJSON_String or is cJSON_IsReference, it should not set valuestring */ + if (!(object->type & cJSON_String) || (object->type & cJSON_IsReference)) { return NULL; } - length = strlen(valuestring) + sizeof(""); - copy = (char*) cJSON_malloc(length); + if (strlen(valuestring) <= strlen(object->valuestring)) + { + strcpy(object->valuestring, valuestring); + return object->valuestring; + } + copy = (char*) cJSON_strdup((const unsigned char*)valuestring, &global_hooks); if (copy == NULL) { return NULL; } - memcpy(copy, valuestring, length); - if (!(object->type & cJSON_IsReference) && (object->valuestring != NULL)) + if (object->valuestring != NULL) { cJSON_free(object->valuestring); } diff --git a/cJSON.h b/cJSON.h index 1455ece..52ad8b4 100644 --- a/cJSON.h +++ b/cJSON.h @@ -278,8 +278,8 @@ CJSON_PUBLIC(cJSON*) cJSON_AddArrayToObject(cJSON * const object, const char * c /* helper for the cJSON_SetNumberValue macro */ CJSON_PUBLIC(double) cJSON_SetNumberHelper(cJSON *object, double number); #define cJSON_SetNumberValue(object, number) ((object != NULL) ? cJSON_SetNumberHelper(object, (double)number) : (number)) -/* Only takes effect when type of object is JSON_Object */ -CJSON_PUBLIC(char*) cJSON_SetValuestringToObject(cJSON *object, const char *valuestring); +/* Change the valuestring of a cJSON_String object, only takes effect when type of object is cJSON_String */ +CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring); /* Macro for iterating over an array or object */ #define cJSON_ArrayForEach(element, array) for(element = (array != NULL) ? (array)->child : NULL; element != NULL; element = element->next) diff --git a/tests/misc_tests.c b/tests/misc_tests.c index ba494f5..2538e8d 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -587,24 +587,34 @@ static void cjson_delete_item_from_array_should_not_broken_list_structure(void) static void cjson_set_valuestring_to_object_should_not_leak_memory(void) { cJSON *root = cJSON_Parse("{}"); - cJSON *item1 = cJSON_CreateString("valuestring could be changed safely"); - cJSON *item2 = cJSON_CreateStringReference("reference item should be freed by yourself"); - const char *newValuestring = "new valuestring which much longer than previous"; - char *returnValue = NULL; + const char *stringvalue = "valuestring could be changed safely"; + const char *reference_valuestring = "reference item should be freed by yourself"; + const char *short_valuestring = "shorter valuestring"; + const char *long_valuestring = "new valuestring which much longer than previous should be changed safely"; + cJSON *item1 = cJSON_CreateString(stringvalue); + cJSON *item2 = cJSON_CreateStringReference(reference_valuestring); + char *ptr1 = NULL; + char *return_value = NULL; cJSON_AddItemToObject(root, "one", item1); cJSON_AddItemToObject(root, "two", item2); - /* we needn't to free the original valuestring manually */ - returnValue = cJSON_SetValuestringToObject(cJSON_GetObjectItem(root, "one"), newValuestring); - TEST_ASSERT_NOT_NULL(returnValue); - TEST_ASSERT_EQUAL_STRING(newValuestring, cJSON_GetObjectItem(root, "one")->valuestring); + ptr1 = item1->valuestring; + return_value = cJSON_SetValuestring(cJSON_GetObjectItem(root, "one"), short_valuestring); + TEST_ASSERT_NOT_NULL(return_value); + TEST_ASSERT_EQUAL_PTR_MESSAGE(ptr1, return_value, "new valuestring shorter than old should not reallocate memory"); + TEST_ASSERT_EQUAL_STRING(short_valuestring, cJSON_GetObjectItem(root, "one")->valuestring); - returnValue = cJSON_SetValuestringToObject(cJSON_GetObjectItem(root, "two"), newValuestring); - TEST_ASSERT_NOT_NULL(returnValue); - TEST_ASSERT_EQUAL_STRING(newValuestring, cJSON_GetObjectItem(root, "two")->valuestring); - /* we must free the memory manually when the item's type is cJSON_IsReference */ - cJSON_free(item2->valuestring); + /* we needn't to free the original valuestring manually */ + ptr1 = item1->valuestring; + return_value = cJSON_SetValuestring(cJSON_GetObjectItem(root, "one"), long_valuestring); + TEST_ASSERT_NOT_NULL(return_value); + TEST_ASSERT_NOT_EQUAL_MESSAGE(ptr1, return_value, "new valuestring longer than old should reallocate memory") + TEST_ASSERT_EQUAL_STRING(long_valuestring, cJSON_GetObjectItem(root, "one")->valuestring); + + return_value = cJSON_SetValuestring(cJSON_GetObjectItem(root, "two"), long_valuestring); + TEST_ASSERT_NULL_MESSAGE(return_value, "valuestring of reference object should not be changed"); + TEST_ASSERT_EQUAL_STRING(reference_valuestring, cJSON_GetObjectItem(root, "two")->valuestring); cJSON_Delete(root); }