From 691a83a479d5512c1067649f799a07b14870faf4 Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Sat, 3 Feb 2018 03:49:00 +0100 Subject: [PATCH] cJSON_CreateConfig: Don't allow configuration, always use default --- cJSON.c | 64 ++++++++++++--------------- cJSON.h | 34 +++------------ tests/configuration_tests.c | 87 +++++++++++++++++++------------------ tests/misc_tests.c | 2 +- 4 files changed, 80 insertions(+), 107 deletions(-) diff --git a/cJSON.c b/cJSON.c index a54d7f7..e73a487 100644 --- a/cJSON.c +++ b/cJSON.c @@ -149,20 +149,32 @@ static cJSON_Hooks global_allocators = { }; /* wrappers around global old style allocators */ -static void *global_allocate_wrapper(size_t size, void *userdata) +static void *global_allocate(size_t size, void *userdata) { (void)userdata; return global_allocators.malloc_fn(size); } -static void *global_reallocate_wrapper(void *pointer, size_t size, void *userdata) +static void global_deallocate(void *pointer, void *userdata) +{ + (void)userdata; + free(pointer); +} + +/* wrappers around standard allocators */ +static void *malloc_wrapper(size_t size, void *userdata) +{ + (void)userdata; + return malloc(size); +} +static void *realloc_wrapper(void *pointer, size_t size, void *userdata) { (void)userdata; return realloc(pointer, size); } -static void global_deallocate_wrapper(void *pointer, void *userdata) +static void free_wrapper(void *pointer, void *userdata) { (void)userdata; - global_allocators.free_fn(pointer); + free(pointer); } /* helpers to allocate memory from a configuration */ @@ -185,9 +197,9 @@ static void deallocate(const internal_configuration * const configuration, void true, /* allow data after the JSON by default */\ true, /* case sensitive by default */\ {\ - global_allocate_wrapper,\ - global_deallocate_wrapper,\ - global_reallocate_wrapper\ + malloc_wrapper,\ + free_wrapper,\ + realloc_wrapper\ },\ NULL, /* no userdata */\ NULL /* no end position */\ @@ -221,17 +233,12 @@ static unsigned char* custom_strdup(const unsigned char* string, const internal_ CJSON_PUBLIC(void) cJSON_InitHooks(cJSON_Hooks* hooks) { - /* set the wrappers in the global configuration */ - global_configuration.userdata = NULL; - global_configuration.allocators.allocate = global_allocate_wrapper; - global_configuration.allocators.deallocate = global_deallocate_wrapper; - global_configuration.allocators.reallocate = global_reallocate_wrapper; - if (hooks == NULL) { - /* reset global allocators */ - global_allocators.malloc_fn = internal_malloc; - global_allocators.free_fn = internal_free; + /* reset global configuration */ + global_configuration.allocators.allocate = malloc_wrapper; + global_configuration.allocators.deallocate = free_wrapper; + global_configuration.allocators.reallocate = realloc_wrapper; return; } @@ -248,12 +255,10 @@ CJSON_PUBLIC(void) cJSON_InitHooks(cJSON_Hooks* hooks) global_allocators.free_fn = hooks->free_fn; } - /* use realloc only if both free and malloc are used */ + /* set the wrappers in the global configuration */ + global_configuration.allocators.allocate = global_allocate; + global_configuration.allocators.deallocate = global_deallocate; global_configuration.allocators.reallocate = NULL; - if ((hooks->malloc_fn == malloc) && (hooks->free_fn == free)) - { - global_configuration.allocators.reallocate = global_reallocate_wrapper; - } } /* Internal constructor. */ @@ -2889,7 +2894,7 @@ CJSON_PUBLIC(cJSON_bool) cJSON_IsRaw(const cJSON * const item) return (item->type & 0xFF) == cJSON_Raw; } -CJSON_PUBLIC(cJSON_Configuration) cJSON_CreateConfiguration(const cJSON * const json, const cJSON_Allocators * const allocators, void *allocator_userdata) +CJSON_PUBLIC(cJSON_Configuration) cJSON_CreateConfiguration(const cJSON_Allocators * const allocators, void *allocator_userdata) { internal_configuration *configuration = NULL; const cJSON_Allocators *local_allocators = &global_configuration.allocators; @@ -2898,17 +2903,12 @@ CJSON_PUBLIC(cJSON_Configuration) cJSON_CreateConfiguration(const cJSON * const { if ((allocators->allocate == NULL) || (allocators->deallocate == NULL)) { - goto fail; + return NULL; } local_allocators = allocators; } - if ((json != NULL) && !cJSON_IsObject(json)) - { - goto fail; - } - configuration = (internal_configuration*)local_allocators->allocate(sizeof(internal_configuration), allocator_userdata); if (configuration == NULL) { @@ -2917,14 +2917,6 @@ CJSON_PUBLIC(cJSON_Configuration) cJSON_CreateConfiguration(const cJSON * const /* initialize with the default */ *configuration = global_default_configuration; - configuration->userdata = allocator_userdata; - configuration->allocators = *local_allocators; - - if (json == NULL) - { - /* default configuration */ - return configuration; - } return (cJSON_Configuration)configuration; diff --git a/cJSON.h b/cJSON.h index 21a6a43..d423d1b 100644 --- a/cJSON.h +++ b/cJSON.h @@ -141,39 +141,19 @@ then using the CJSON_API_VISIBILITY flag to "export" the same symbols the way CJ /* returns the version of cJSON as a string */ CJSON_PUBLIC(const char*) cJSON_Version(void); -/* Create a configuration object that can be passed to several functions - * to configure their behavior. - * A configuration is given in JSON form (case sensitive) and can optionally contain any - * of the following options: - * - buffer_size: number of bytes that the printbuffer should be initially - * - format: boolean that indicates if the output should be formatted - * - case_sensitive: boolean that indicates if object keys should be considered case_sensitive - * - allow_data_after_json: boolean that indicates if parsing succeeds if the JSON in the - * input is followed by non JSON data - * - * - * If NULL is passed to a function that expects an object of type cJSON_Configuration, - * the following default configuration is used: - * { - * "format": true, - * "case_sensitive": true, - * "allow_data_after_json": true - * } +/* + * Create a configuration object that can be passed to several functions + * to configure their behavior. It will be set to the default values initially, they + * can be changed later. * * A cJSON_Configuration object is dynamically allocated and you are responsible to free it * after use. * - * If allocators is a NULL pointer, the global default allocators are used (the one that is set - * by cJSON_InitHooks, malloc/free by default). - * The allocator is automatically attached to the configuration, so it will be used by functions - * that the configuration is passed to. This can be changed later with - * cJSON_ConfigurationChangeAllocator. + * If allocators is a NULL pointer, malloc and free are used. * - * allocator_userdata can be used to pass custom data to your allocator. It also gets attached to - * the configuration automatically. This can later be changed with - * cJSON_ConfigurationChangeUserdata. + * allocator_userdata can be used to pass custom data to your allocator (e.g. for pool allocators). * */ -CJSON_PUBLIC(cJSON_Configuration) cJSON_CreateConfiguration(const cJSON * const json, const cJSON_Allocators * const allocators, void *allocator_userdata); +CJSON_PUBLIC(cJSON_Configuration) cJSON_CreateConfiguration(const cJSON_Allocators * const allocators, void *allocator_userdata); /* Change the allocators of a cJSON_Configuration and reset the userdata */ CJSON_PUBLIC(cJSON_Configuration) cJSON_ConfigurationChangeAllocators(cJSON_Configuration configuration, const cJSON_Allocators allocators); /* Change the allocator userdata attached to a cJSON_Configuration */ diff --git a/tests/configuration_tests.c b/tests/configuration_tests.c index 1587e8e..2afe7e6 100644 --- a/tests/configuration_tests.c +++ b/tests/configuration_tests.c @@ -30,43 +30,18 @@ static void create_configuration_should_create_a_configuration(void) { - cJSON *json = NULL; internal_configuration *configuration = NULL; - int userdata = 1; - json = cJSON_Parse("{\"allow_data_after_json\":false}"); - TEST_ASSERT_NOT_NULL(json); - configuration = (internal_configuration*)cJSON_CreateConfiguration(json, NULL, &userdata); - cJSON_Delete(json); - json = NULL; + configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); TEST_ASSERT_EQUAL_MESSAGE(configuration->buffer_size, 256, "buffer_size has an incorrect value."); TEST_ASSERT_TRUE_MESSAGE(configuration->format, "format has an incorrect value."); TEST_ASSERT_TRUE_MESSAGE(configuration->case_sensitive, "case_sensitive has an incorrect value."); TEST_ASSERT_TRUE_MESSAGE(configuration->allow_data_after_json, "allow_data_after_json has an incorrect value."); - TEST_ASSERT_TRUE_MESSAGE(configuration->userdata == &userdata, "Incorrect userdata"); - TEST_ASSERT_TRUE_MESSAGE(global_allocate_wrapper == configuration->allocators.allocate, "Wrong malloc."); - TEST_ASSERT_TRUE_MESSAGE(global_reallocate_wrapper == configuration->allocators.reallocate, "Wrong realloc."); - TEST_ASSERT_TRUE_MESSAGE(global_deallocate_wrapper == configuration->allocators.deallocate, "Wrong realloc."); - - free(configuration); -} - -static void create_configuration_should_work_with_an_empty_object(void) -{ - internal_configuration *configuration = NULL; - int userdata = 1; - - configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, &userdata); - TEST_ASSERT_NOT_NULL(configuration); - TEST_ASSERT_EQUAL_MESSAGE(configuration->buffer_size, 256, "buffer_size has an incorrect value."); - TEST_ASSERT_TRUE_MESSAGE(configuration->format, "format has an incorrect value."); - TEST_ASSERT_TRUE_MESSAGE(configuration->case_sensitive, "case_sensitive has an incorrect value."); - TEST_ASSERT_TRUE_MESSAGE(configuration->allow_data_after_json, "allow_data_after_json has an incorrect value."); - TEST_ASSERT_TRUE_MESSAGE(configuration->userdata == &userdata, "Incorrect userdata"); - TEST_ASSERT_TRUE_MESSAGE(global_allocate_wrapper == configuration->allocators.allocate, "Wrong malloc."); - TEST_ASSERT_TRUE_MESSAGE(global_reallocate_wrapper == configuration->allocators.reallocate, "Wrong realloc."); - TEST_ASSERT_TRUE_MESSAGE(global_deallocate_wrapper == configuration->allocators.deallocate, "Wrong free."); + TEST_ASSERT_NULL_MESSAGE(configuration->userdata, "Userdata should be NULL"); + TEST_ASSERT_TRUE_MESSAGE(malloc_wrapper == configuration->allocators.allocate, "Wrong malloc."); + TEST_ASSERT_TRUE_MESSAGE(realloc_wrapper == configuration->allocators.reallocate, "Wrong realloc."); + TEST_ASSERT_TRUE_MESSAGE(free_wrapper == configuration->allocators.deallocate, "Wrong free."); free(configuration); } @@ -88,23 +63,33 @@ static void create_configuration_should_take_custom_allocators(void) cJSON_Allocators allocators = {custom_allocator, custom_deallocator, NULL}; size_t userdata = 0; - configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, &allocators, &userdata); + configuration = (internal_configuration*)cJSON_CreateConfiguration(&allocators, &userdata); TEST_ASSERT_NOT_NULL(configuration); TEST_ASSERT_EQUAL_MESSAGE(userdata, sizeof(internal_configuration), "custom allocator wasn't run properly."); - TEST_ASSERT_TRUE_MESSAGE(custom_allocator == configuration->allocators.allocate, "Wrong allocator."); - TEST_ASSERT_TRUE_MESSAGE(custom_deallocator == configuration->allocators.deallocate, "Wrong deallocator."); - TEST_ASSERT_NULL_MESSAGE(configuration->allocators.reallocate, "Reallocator is not null"); + TEST_ASSERT_TRUE_MESSAGE(global_default_configuration.allocators.allocate == configuration->allocators.allocate, "Wrong allocator."); + TEST_ASSERT_TRUE_MESSAGE(global_default_configuration.allocators.deallocate == configuration->allocators.deallocate, "Wrong deallocator."); + TEST_ASSERT_TRUE_MESSAGE(global_default_configuration.allocators.reallocate == configuration->allocators.reallocate, "Wrong reallocator."); custom_deallocator(configuration, &userdata); } +static void create_configuration_should_not_take_incomplete_allocators(void) +{ + cJSON_Allocators allocators1 = {custom_allocator, NULL, NULL}; + cJSON_Allocators allocators2 = {NULL, custom_deallocator, NULL}; + size_t userdata = 0; + + TEST_ASSERT_NULL(cJSON_CreateConfiguration(&allocators1, &userdata)); + TEST_ASSERT_NULL(cJSON_CreateConfiguration(&allocators2, &userdata)); +} + static void configuration_change_allocators_should_change_allocators(void) { internal_configuration *configuration = NULL; cJSON_Allocators allocators = {custom_allocator, custom_deallocator, NULL}; size_t userdata = 0; - configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, &allocators, &userdata); + configuration = (internal_configuration*)cJSON_CreateConfiguration(&allocators, &userdata); TEST_ASSERT_NOT_NULL(configuration); configuration = (internal_configuration*)cJSON_ConfigurationChangeAllocators(configuration, allocators); @@ -116,11 +101,26 @@ static void configuration_change_allocators_should_change_allocators(void) custom_deallocator(configuration, &userdata); } +static void configuration_change_allocators_should_not_change_incomplete_allocators(void) +{ + internal_configuration *configuration = NULL; + cJSON_Allocators allocators1 = {custom_allocator, NULL, NULL}; + cJSON_Allocators allocators2 = {NULL, custom_deallocator, NULL}; + + configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); + TEST_ASSERT_NOT_NULL(configuration); + + TEST_ASSERT_NULL(cJSON_ConfigurationChangeAllocators(configuration, allocators1)); + TEST_ASSERT_NULL(cJSON_ConfigurationChangeAllocators(configuration, allocators2)); + + free(configuration); +} + static void configuration_change_userdata_should_change_userdata(void) { internal_configuration *configuration = NULL; size_t userdata = 0; - configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, NULL); + configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); configuration = (internal_configuration*)cJSON_ConfigurationChangeUserdata(configuration, &userdata); @@ -132,7 +132,7 @@ static void configuration_change_userdata_should_change_userdata(void) static void configuration_change_parse_end_should_change_parse_end(void) { size_t end_position = 0; - internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, NULL); + internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); configuration = (internal_configuration*)cJSON_ConfigurationChangeParseEnd(configuration, &end_position); @@ -145,7 +145,7 @@ static void configuration_change_parse_end_should_change_parse_end(void) static void configuration_change_prebuffer_size_should_change_buffer_size(void) { - internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, NULL); + internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); configuration = (internal_configuration*)cJSON_ConfigurationChangePrebufferSize(configuration, 1024); @@ -158,7 +158,7 @@ static void configuration_change_prebuffer_size_should_change_buffer_size(void) static void configuration_change_prebuffer_size_should_not_allow_empty_sizes(void) { - internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, NULL); + internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); TEST_ASSERT_NULL(cJSON_ConfigurationChangePrebufferSize(configuration, 0)); @@ -168,7 +168,7 @@ static void configuration_change_prebuffer_size_should_not_allow_empty_sizes(voi static void configuration_change_format_should_change_format(void) { - internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, NULL); + internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); configuration = (internal_configuration*)cJSON_ConfigurationChangeFormat(configuration, CJSON_FORMAT_MINIFIED); @@ -186,7 +186,7 @@ static void configuration_change_format_should_change_format(void) static void configuration_change_case_sensitivity_should_change_case_sensitivity(void) { - internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, NULL); + internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); configuration = (internal_configuration*)cJSON_ConfigurationChangeCaseSensitivity(configuration, false); @@ -199,7 +199,7 @@ static void configuration_change_case_sensitivity_should_change_case_sensitivity static void configuration_change_allow_data_after_json_should_change_allow_data_after_json(void) { - internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL, NULL); + internal_configuration *configuration = (internal_configuration*)cJSON_CreateConfiguration(NULL, NULL); TEST_ASSERT_NOT_NULL(configuration); configuration = (internal_configuration*)cJSON_ConfigurationChangeAllowDataAfterJson(configuration, false); @@ -215,9 +215,10 @@ int main(void) UNITY_BEGIN(); RUN_TEST(create_configuration_should_create_a_configuration); - RUN_TEST(create_configuration_should_work_with_an_empty_object); RUN_TEST(create_configuration_should_take_custom_allocators); + RUN_TEST(create_configuration_should_not_take_incomplete_allocators); RUN_TEST(configuration_change_allocators_should_change_allocators); + RUN_TEST(configuration_change_allocators_should_not_change_incomplete_allocators); RUN_TEST(configuration_change_userdata_should_change_userdata); RUN_TEST(configuration_change_parse_end_should_change_parse_end); RUN_TEST(configuration_change_prebuffer_size_should_change_buffer_size); diff --git a/tests/misc_tests.c b/tests/misc_tests.c index 6463639..10c0f92 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -419,7 +419,7 @@ static void *failing_realloc(void *pointer, size_t size, void *userdata) static void ensure_should_fail_on_failed_realloc(void) { - printbuffer buffer = {NULL, 10, 0, 0, false, {256, false, true, true, {global_allocate_wrapper, global_deallocate_wrapper, failing_realloc}, NULL, NULL } }; + printbuffer buffer = {NULL, 10, 0, 0, false, {256, false, true, true, {malloc_wrapper, free_wrapper, failing_realloc}, NULL, NULL } }; buffer.configuration.userdata = &buffer; buffer.buffer = (unsigned char*)malloc(100); TEST_ASSERT_NOT_NULL(buffer.buffer);