diff --git a/include/cfl/cfl_array.h b/include/cfl/cfl_array.h index 8092344..f057bf4 100644 --- a/include/cfl/cfl_array.h +++ b/include/cfl/cfl_array.h @@ -33,6 +33,9 @@ struct cfl_array { struct cfl_variant **entries; size_t slot_count; size_t entry_count; + struct cfl_variant *owner; + struct cfl_array *parent_array; + struct cfl_kvlist *parent_kvlist; }; struct cfl_array *cfl_array_create(size_t slot_count); @@ -62,9 +65,10 @@ static inline size_t cfl_array_size(struct cfl_array *array) } /* - * Append APIs take ownership of the value on success. A variant, array, or - * kvlist must have a single owning parent; inserting the same pointer into - * multiple containers is unsupported and can result in double-free. + * Append APIs take ownership of the value on success. A raw array or kvlist + * must have one owning variant at a time. To move an existing kvpair value, + * detach it with cfl_kvpair_take_value() before reinserting it. Do not leave + * the same variant pointer attached to multiple live containers. */ int cfl_array_append(struct cfl_array *array, struct cfl_variant *value); int cfl_array_append_string(struct cfl_array *array, char *value); diff --git a/include/cfl/cfl_container.h b/include/cfl/cfl_container.h index 6583e59..714df85 100644 --- a/include/cfl/cfl_container.h +++ b/include/cfl/cfl_container.h @@ -45,4 +45,15 @@ int cfl_container_variant_contains_kvlist(struct cfl_variant *variant, int cfl_container_variant_contains_variant(struct cfl_variant *variant, struct cfl_variant *target); +int cfl_container_claim_array(struct cfl_array *array, + struct cfl_variant *owner); +int cfl_container_claim_kvlist(struct cfl_kvlist *kvlist, + struct cfl_variant *owner); +int cfl_container_adopt_variant(struct cfl_variant *variant); +int cfl_container_move_variant_to_array(struct cfl_array *array, + struct cfl_variant *variant); +int cfl_container_move_variant_to_kvlist(struct cfl_kvlist *kvlist, + struct cfl_variant *variant); +void cfl_container_release_variant(struct cfl_variant *variant); + #endif diff --git a/include/cfl/cfl_kvlist.h b/include/cfl/cfl_kvlist.h index b2cb2a4..cd8d904 100644 --- a/include/cfl/cfl_kvlist.h +++ b/include/cfl/cfl_kvlist.h @@ -28,6 +28,8 @@ #include #include +struct cfl_array; + struct cfl_kvpair { cfl_sds_t key; /* Key */ struct cfl_variant *val; /* Value */ @@ -35,7 +37,10 @@ struct cfl_kvpair { }; struct cfl_kvlist { - struct cfl_list list; + struct cfl_list list; + struct cfl_variant *owner; + struct cfl_array *parent_array; + struct cfl_kvlist *parent_kvlist; }; struct cfl_kvlist *cfl_kvlist_create(); @@ -43,8 +48,10 @@ void cfl_kvlist_destroy(struct cfl_kvlist *list); /* * Insert APIs take ownership of array, kvlist, and variant values on success. - * A value must have a single owning parent; inserting the same pointer into - * multiple containers is unsupported and can result in double-free. + * A raw array or kvlist must have one owning variant at a time. To move an + * existing kvpair value, detach it with cfl_kvpair_take_value() before + * reinserting it. Do not leave the same variant pointer attached to multiple + * live containers. */ int cfl_kvlist_insert_string(struct cfl_kvlist *list, char *key, char *value); @@ -136,6 +143,7 @@ struct cfl_variant *cfl_kvlist_fetch_s(struct cfl_kvlist *list, char *key, size_ int cfl_kvlist_contains(struct cfl_kvlist *kvlist, char *name); int cfl_kvlist_remove(struct cfl_kvlist *kvlist, char *name); void cfl_kvpair_destroy(struct cfl_kvpair *pair); +struct cfl_variant *cfl_kvpair_take_value(struct cfl_kvpair *pair); #endif diff --git a/include/cfl/cfl_variant.h b/include/cfl/cfl_variant.h index 685f51a..efdcfe9 100644 --- a/include/cfl/cfl_variant.h +++ b/include/cfl/cfl_variant.h @@ -56,6 +56,7 @@ struct cfl_variant { * a copy of the original data. */ uint8_t referenced; + uint8_t owned; /* the data */ union { diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 62c6cff..5b08ec2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,3 +1,5 @@ +include(CheckCSourceCompiles) + set(src cfl.c cfl_log.c @@ -14,20 +16,63 @@ set(src ) set(CFL_ATOMIC_NEEDS_THREADS Off) +set(CFL_ATOMIC_NEEDS_LIBATOMIC Off) +set(CFL_ATOMIC_USES_BUILTINS Off) + +set(CFL_ATOMIC_BUILTINS_LINK_SOURCE " + #include + int main(void) { + uint64_t storage = 0; + uint64_t expected = 0; + uint64_t desired = 1; + __atomic_store_n(&storage, desired, __ATOMIC_SEQ_CST); + (void) __atomic_load_n(&storage, __ATOMIC_SEQ_CST); + (void) __atomic_compare_exchange(&storage, &expected, &desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); + return 0; + }") if(MSVC) set(PLATFORM_SPECIFIC_ATOMIC_MODULE cfl_atomic_msvc.c) elseif("${CMAKE_C_COMPILER_ID}" STREQUAL "Clang") set(PLATFORM_SPECIFIC_ATOMIC_MODULE cfl_atomic_clang.c) + set(CFL_ATOMIC_USES_BUILTINS On) elseif("${CMAKE_C_COMPILER_ID}" STREQUAL "AppleClang") set(PLATFORM_SPECIFIC_ATOMIC_MODULE cfl_atomic_clang.c) + set(CFL_ATOMIC_USES_BUILTINS On) elseif("${CMAKE_C_COMPILER_ID}" STREQUAL "GNU") set(PLATFORM_SPECIFIC_ATOMIC_MODULE cfl_atomic_gcc.c) + set(CFL_ATOMIC_USES_BUILTINS On) else() set(PLATFORM_SPECIFIC_ATOMIC_MODULE cfl_atomic_generic.c) set(CFL_ATOMIC_NEEDS_THREADS On) endif() +if(CFL_ATOMIC_USES_BUILTINS) + check_c_source_compiles("${CFL_ATOMIC_BUILTINS_LINK_SOURCE}" + CFL_ATOMIC_BUILTINS_LINK) + + if(NOT CFL_ATOMIC_BUILTINS_LINK) + set(CFL_ATOMIC_REQUIRED_LIBRARIES "${CMAKE_REQUIRED_LIBRARIES}") + set(CMAKE_REQUIRED_LIBRARIES ${CFL_ATOMIC_REQUIRED_LIBRARIES}) + list(APPEND CMAKE_REQUIRED_LIBRARIES atomic) + + check_c_source_compiles("${CFL_ATOMIC_BUILTINS_LINK_SOURCE}" + CFL_ATOMIC_BUILTINS_LINK_WITH_LIBATOMIC) + + set(CMAKE_REQUIRED_LIBRARIES "${CFL_ATOMIC_REQUIRED_LIBRARIES}") + + if(CFL_ATOMIC_BUILTINS_LINK_WITH_LIBATOMIC) + set(CFL_ATOMIC_NEEDS_LIBATOMIC On) + elseif(CFL_SYSTEM_WINDOWS) + set(PLATFORM_SPECIFIC_ATOMIC_MODULE cfl_atomic_msvc.c) + else() + set(PLATFORM_SPECIFIC_ATOMIC_MODULE cfl_atomic_generic.c) + set(CFL_ATOMIC_NEEDS_THREADS On) + endif() + endif() +endif() + set(src ${src} ${PLATFORM_SPECIFIC_ATOMIC_MODULE} @@ -37,6 +82,10 @@ set(src add_library(cfl-static STATIC ${src}) target_link_libraries(cfl-static PRIVATE xxhash) +if(CFL_ATOMIC_NEEDS_LIBATOMIC) + target_link_libraries(cfl-static PUBLIC atomic) +endif() + if(CFL_ATOMIC_NEEDS_THREADS) find_package(Threads REQUIRED) target_link_libraries(cfl-static PUBLIC Threads::Threads) diff --git a/src/cfl_array.c b/src/cfl_array.c index 10039df..af9d7cd 100644 --- a/src/cfl_array.c +++ b/src/cfl_array.c @@ -58,6 +58,9 @@ struct cfl_array *cfl_array_create(size_t slot_count) array->entry_count = 0; array->slot_count = slot_count; + array->owner = NULL; + array->parent_array = NULL; + array->parent_kvlist = NULL; return array; } @@ -152,27 +155,6 @@ int cfl_array_append(struct cfl_array *array, return -1; } - if (cfl_container_array_contains_variant(array, value)) { - return -1; - } - - /* Only container-valued variants can participate in container cycles. */ - if (value->type == CFL_VARIANT_ARRAY || value->type == CFL_VARIANT_KVLIST) { - if (cfl_container_variant_contains_array(value, array)) { - return -1; - } - - if (value->type == CFL_VARIANT_ARRAY && - cfl_container_array_contains_array(array, value->data.as_array)) { - return -1; - } - - if (value->type == CFL_VARIANT_KVLIST && - cfl_container_array_contains_kvlist(array, value->data.as_kvlist)) { - return -1; - } - } - if (array->entry_count >= array->slot_count) { /* * if there is no more space but the caller allowed to resize @@ -216,6 +198,10 @@ int cfl_array_append(struct cfl_array *array, return -1; } + if (cfl_container_move_variant_to_array(array, value) != 0) { + return -1; + } + array->entries[array->entry_count++] = value; return 0; } @@ -420,8 +406,7 @@ int cfl_array_append_array(struct cfl_array *array, struct cfl_array *value) return -1; } - if (cfl_container_array_contains_array(array, value) || - cfl_container_array_contains_array(value, array)) { + if (array == value) { return -1; } @@ -433,6 +418,8 @@ int cfl_array_append_array(struct cfl_array *array, struct cfl_array *value) result = cfl_array_append(array, value_instance); if (result) { + cfl_container_release_variant(value_instance); + value_instance->data.as_array = NULL; cfl_variant_destroy(value_instance); return -2; } @@ -457,7 +444,7 @@ int cfl_array_append_new_array(struct cfl_array *array, size_t size) } result = cfl_array_append_array(array, value); - if (result == -1) { + if (result < 0) { cfl_array_destroy(value); } @@ -473,11 +460,6 @@ int cfl_array_append_kvlist(struct cfl_array *array, struct cfl_kvlist *value) return -1; } - if (cfl_container_array_contains_kvlist(array, value) || - cfl_container_kvlist_contains_array(value, array)) { - return -1; - } - value_instance = cfl_variant_create_from_kvlist(value); if (value_instance == NULL) { return -1; @@ -485,6 +467,8 @@ int cfl_array_append_kvlist(struct cfl_array *array, struct cfl_kvlist *value) result = cfl_array_append(array, value_instance); if (result) { + cfl_container_release_variant(value_instance); + value_instance->data.as_kvlist = NULL; cfl_variant_destroy(value_instance); return -2; diff --git a/src/cfl_container.c b/src/cfl_container.c index 5bf7a23..3b34633 100644 --- a/src/cfl_container.c +++ b/src/cfl_container.c @@ -323,3 +323,271 @@ int cfl_container_variant_contains_variant(struct cfl_variant *variant, { return variant_contains_variant(variant, target, 0); } + +static int parent_chain_contains_array(struct cfl_array *array, + struct cfl_kvlist *kvlist, + struct cfl_array *target) +{ + struct cfl_array *next_array; + struct cfl_kvlist *next_kvlist; + size_t depth; + + depth = 0; + + while (array != NULL || kvlist != NULL) { + if (depth_exceeded(depth)) { + return CFL_TRUE; + } + + next_array = NULL; + next_kvlist = NULL; + + if (array != NULL) { + if (array == target) { + return CFL_TRUE; + } + + next_array = array->parent_array; + next_kvlist = array->parent_kvlist; + } + else { + next_array = kvlist->parent_array; + next_kvlist = kvlist->parent_kvlist; + } + + array = next_array; + kvlist = next_kvlist; + depth++; + } + + return CFL_FALSE; +} + +static int parent_chain_contains_kvlist(struct cfl_array *array, + struct cfl_kvlist *kvlist, + struct cfl_kvlist *target) +{ + struct cfl_array *next_array; + struct cfl_kvlist *next_kvlist; + size_t depth; + + depth = 0; + + while (array != NULL || kvlist != NULL) { + if (depth_exceeded(depth)) { + return CFL_TRUE; + } + + next_array = NULL; + next_kvlist = NULL; + + if (kvlist != NULL) { + if (kvlist == target) { + return CFL_TRUE; + } + + next_array = kvlist->parent_array; + next_kvlist = kvlist->parent_kvlist; + } + else { + next_array = array->parent_array; + next_kvlist = array->parent_kvlist; + } + + array = next_array; + kvlist = next_kvlist; + depth++; + } + + return CFL_FALSE; +} + +static int claim_variant_container(struct cfl_variant *variant) +{ + if (variant->type == CFL_VARIANT_ARRAY) { + if (variant->data.as_array != NULL && + cfl_container_claim_array(variant->data.as_array, variant) != 0) { + return -1; + } + } + + if (variant->type == CFL_VARIANT_KVLIST) { + if (variant->data.as_kvlist != NULL && + cfl_container_claim_kvlist(variant->data.as_kvlist, variant) != 0) { + return -1; + } + } + + return 0; +} + +int cfl_container_claim_array(struct cfl_array *array, + struct cfl_variant *owner) +{ + if (array == NULL || owner == NULL) { + return -1; + } + + if (array->owner != NULL && array->owner != owner) { + return -1; + } + + array->owner = owner; + + return 0; +} + +int cfl_container_claim_kvlist(struct cfl_kvlist *kvlist, + struct cfl_variant *owner) +{ + if (kvlist == NULL || owner == NULL) { + return -1; + } + + if (kvlist->owner != NULL && kvlist->owner != owner) { + return -1; + } + + kvlist->owner = owner; + + return 0; +} + +int cfl_container_adopt_variant(struct cfl_variant *variant) +{ + if (variant == NULL) { + return -1; + } + + if (variant->owned) { + return -1; + } + + if (claim_variant_container(variant) != 0) { + return -1; + } + + variant->owned = CFL_TRUE; + + return 0; +} + +int cfl_container_move_variant_to_array(struct cfl_array *array, + struct cfl_variant *variant) +{ + struct cfl_array *child_array; + struct cfl_kvlist *child_kvlist; + + if (array == NULL || variant == NULL) { + return -1; + } + + if (variant->type == CFL_VARIANT_ARRAY) { + child_array = variant->data.as_array; + + if (child_array != NULL && + parent_chain_contains_array(array, NULL, child_array)) { + return -1; + } + } + else if (variant->type == CFL_VARIANT_KVLIST) { + child_kvlist = variant->data.as_kvlist; + + if (child_kvlist != NULL && + parent_chain_contains_kvlist(array, NULL, child_kvlist)) { + return -1; + } + } + + if (claim_variant_container(variant) != 0) { + return -1; + } + + if (variant->type == CFL_VARIANT_ARRAY && + variant->data.as_array != NULL) { + variant->data.as_array->parent_array = array; + variant->data.as_array->parent_kvlist = NULL; + } + else if (variant->type == CFL_VARIANT_KVLIST && + variant->data.as_kvlist != NULL) { + variant->data.as_kvlist->parent_array = array; + variant->data.as_kvlist->parent_kvlist = NULL; + } + + variant->owned = CFL_TRUE; + + return 0; +} + +int cfl_container_move_variant_to_kvlist(struct cfl_kvlist *kvlist, + struct cfl_variant *variant) +{ + struct cfl_array *child_array; + struct cfl_kvlist *child_kvlist; + + if (kvlist == NULL || variant == NULL) { + return -1; + } + + if (variant->type == CFL_VARIANT_ARRAY) { + child_array = variant->data.as_array; + + if (child_array != NULL && + parent_chain_contains_array(NULL, kvlist, child_array)) { + return -1; + } + } + else if (variant->type == CFL_VARIANT_KVLIST) { + child_kvlist = variant->data.as_kvlist; + + if (child_kvlist != NULL && + parent_chain_contains_kvlist(NULL, kvlist, child_kvlist)) { + return -1; + } + } + + if (claim_variant_container(variant) != 0) { + return -1; + } + + if (variant->type == CFL_VARIANT_ARRAY && + variant->data.as_array != NULL) { + variant->data.as_array->parent_array = NULL; + variant->data.as_array->parent_kvlist = kvlist; + } + else if (variant->type == CFL_VARIANT_KVLIST && + variant->data.as_kvlist != NULL) { + variant->data.as_kvlist->parent_array = NULL; + variant->data.as_kvlist->parent_kvlist = kvlist; + } + + variant->owned = CFL_TRUE; + + return 0; +} + +void cfl_container_release_variant(struct cfl_variant *variant) +{ + if (variant == NULL) { + return; + } + + variant->owned = CFL_FALSE; + + if (variant->type == CFL_VARIANT_ARRAY) { + if (variant->data.as_array != NULL && + variant->data.as_array->owner == variant) { + variant->data.as_array->owner = NULL; + variant->data.as_array->parent_array = NULL; + variant->data.as_array->parent_kvlist = NULL; + } + } + else if (variant->type == CFL_VARIANT_KVLIST) { + if (variant->data.as_kvlist != NULL && + variant->data.as_kvlist->owner == variant) { + variant->data.as_kvlist->owner = NULL; + variant->data.as_kvlist->parent_array = NULL; + variant->data.as_kvlist->parent_kvlist = NULL; + } + } +} diff --git a/src/cfl_kvlist.c b/src/cfl_kvlist.c index a13d56c..9d1bb18 100644 --- a/src/cfl_kvlist.c +++ b/src/cfl_kvlist.c @@ -95,6 +95,10 @@ struct cfl_kvlist *cfl_kvlist_create() } cfl_list_init(&list->list); + list->owner = NULL; + list->parent_array = NULL; + list->parent_kvlist = NULL; + return list; } @@ -324,11 +328,6 @@ int cfl_kvlist_insert_array_s(struct cfl_kvlist *list, return -1; } - if (cfl_container_kvlist_contains_array(list, value) || - cfl_container_array_contains_kvlist(value, list)) { - return -1; - } - value_instance = cfl_variant_create_from_array(value); if (value_instance == NULL) { @@ -338,6 +337,8 @@ int cfl_kvlist_insert_array_s(struct cfl_kvlist *list, result = cfl_kvlist_insert_s(list, key, key_size, value_instance); if (result) { + cfl_container_release_variant(value_instance); + value_instance->data.as_array = NULL; cfl_variant_destroy(value_instance); return -2; @@ -364,7 +365,7 @@ int cfl_kvlist_insert_new_array_s(struct cfl_kvlist *list, } result = cfl_kvlist_insert_array_s(list, key, key_size, value); - if (result == -1) { + if (result < 0) { cfl_array_destroy(value); } @@ -381,8 +382,7 @@ int cfl_kvlist_insert_kvlist_s(struct cfl_kvlist *list, return -1; } - if (cfl_container_kvlist_contains_kvlist(list, value) || - cfl_container_kvlist_contains_kvlist(value, list)) { + if (list == value) { return -1; } @@ -394,6 +394,8 @@ int cfl_kvlist_insert_kvlist_s(struct cfl_kvlist *list, result = cfl_kvlist_insert_s(list, key, key_size, value_instance); if (result) { + cfl_container_release_variant(value_instance); + value_instance->data.as_kvlist = NULL; cfl_variant_destroy(value_instance); return -2; @@ -412,27 +414,6 @@ int cfl_kvlist_insert_s(struct cfl_kvlist *list, return -1; } - if (cfl_container_kvlist_contains_variant(list, value)) { - return -1; - } - - /* Only container-valued variants can participate in container cycles. */ - if (value->type == CFL_VARIANT_ARRAY || value->type == CFL_VARIANT_KVLIST) { - if (cfl_container_variant_contains_kvlist(value, list)) { - return -1; - } - - if (value->type == CFL_VARIANT_ARRAY && - cfl_container_kvlist_contains_array(list, value->data.as_array)) { - return -1; - } - - if (value->type == CFL_VARIANT_KVLIST && - cfl_container_kvlist_contains_kvlist(list, value->data.as_kvlist)) { - return -1; - } - } - pair = malloc(sizeof(struct cfl_kvpair)); if (pair == NULL) { cfl_report_runtime_error(); @@ -446,6 +427,13 @@ int cfl_kvlist_insert_s(struct cfl_kvlist *list, return -2; } + if (cfl_container_move_variant_to_kvlist(list, value) != 0) { + cfl_sds_destroy(pair->key); + free(pair); + + return -1; + } + pair->val = value; cfl_list_add(&pair->_head, &list->list); @@ -754,3 +742,19 @@ void cfl_kvpair_destroy(struct cfl_kvpair *pair) free(pair); } } + +struct cfl_variant *cfl_kvpair_take_value(struct cfl_kvpair *pair) +{ + struct cfl_variant *value; + + if (pair == NULL) { + return NULL; + } + + value = pair->val; + pair->val = NULL; + + cfl_container_release_variant(value); + + return value; +} diff --git a/src/cfl_object.c b/src/cfl_object.c index f840a61..199b62b 100644 --- a/src/cfl_object.c +++ b/src/cfl_object.c @@ -69,126 +69,6 @@ static int reuses_current_root(struct cfl_object *o, int type, void *ptr) return CFL_FALSE; } -static int kvlist_contains_current(struct cfl_kvlist *kvlist, - struct cfl_variant *current) -{ - if (cfl_container_kvlist_contains_variant(kvlist, current)) { - return CFL_TRUE; - } - - if (current->type == CFL_VARIANT_KVLIST && - cfl_container_kvlist_contains_kvlist(kvlist, - current->data.as_kvlist)) { - return CFL_TRUE; - } - - if (current->type == CFL_VARIANT_ARRAY && - cfl_container_kvlist_contains_array(kvlist, - current->data.as_array)) { - return CFL_TRUE; - } - - return CFL_FALSE; -} - -static int array_contains_current(struct cfl_array *array, - struct cfl_variant *current) -{ - if (cfl_container_array_contains_variant(array, current)) { - return CFL_TRUE; - } - - if (current->type == CFL_VARIANT_KVLIST && - cfl_container_array_contains_kvlist(array, - current->data.as_kvlist)) { - return CFL_TRUE; - } - - if (current->type == CFL_VARIANT_ARRAY && - cfl_container_array_contains_array(array, - current->data.as_array)) { - return CFL_TRUE; - } - - return CFL_FALSE; -} - -static int variant_contains_current(struct cfl_variant *variant, - struct cfl_variant *current) -{ - if (cfl_container_variant_contains_variant(variant, current)) { - return CFL_TRUE; - } - - if (current->type == CFL_VARIANT_KVLIST && - cfl_container_variant_contains_kvlist(variant, - current->data.as_kvlist)) { - return CFL_TRUE; - } - - if (current->type == CFL_VARIANT_ARRAY && - cfl_container_variant_contains_array(variant, - current->data.as_array)) { - return CFL_TRUE; - } - - return CFL_FALSE; -} - -static int current_contains_candidate(struct cfl_variant *current, - int type, void *ptr) -{ - struct cfl_variant *variant; - - if (type == CFL_OBJECT_KVLIST) { - return cfl_container_variant_contains_kvlist(current, ptr); - } - - if (type == CFL_OBJECT_ARRAY) { - return cfl_container_variant_contains_array(current, ptr); - } - - if (type == CFL_OBJECT_VARIANT) { - variant = ptr; - - if (cfl_container_variant_contains_variant(current, variant)) { - return CFL_TRUE; - } - - if (variant->type == CFL_VARIANT_KVLIST && - cfl_container_variant_contains_kvlist(current, - variant->data.as_kvlist)) { - return CFL_TRUE; - } - - if (variant->type == CFL_VARIANT_ARRAY && - cfl_container_variant_contains_array(current, - variant->data.as_array)) { - return CFL_TRUE; - } - } - - return CFL_FALSE; -} - -static int candidate_contains_current(struct cfl_variant *current, - int type, void *ptr) -{ - if (type == CFL_OBJECT_KVLIST) { - return kvlist_contains_current(ptr, current); - } - - if (type == CFL_OBJECT_ARRAY) { - return array_contains_current(ptr, current); - } - - if (type == CFL_OBJECT_VARIANT) { - return variant_contains_current(ptr, current); - } - - return CFL_FALSE; -} - /* * Associate a CFL data type to the object. We only support kvlist, array and variant. Note * that everything is held as a variant internally. @@ -196,6 +76,7 @@ static int candidate_contains_current(struct cfl_variant *current, int cfl_object_set(struct cfl_object *o, int type, void *ptr) { struct cfl_variant *variant; + int variant_created; if (!o || !ptr) { return -1; @@ -206,21 +87,20 @@ int cfl_object_set(struct cfl_object *o, int type, void *ptr) o->type = type; return 0; } - - if (current_contains_candidate(o->variant, type, ptr) || - candidate_contains_current(o->variant, type, ptr)) { - return -1; - } } + variant_created = CFL_FALSE; + if (type == CFL_OBJECT_KVLIST) { variant = cfl_variant_create_from_kvlist(ptr); + variant_created = CFL_TRUE; } else if (type == CFL_OBJECT_VARIANT) { variant = ptr; } else if (type == CFL_OBJECT_ARRAY) { variant = cfl_variant_create_from_array(ptr); + variant_created = CFL_TRUE; } else { return -1; @@ -230,6 +110,14 @@ int cfl_object_set(struct cfl_object *o, int type, void *ptr) return -1; } + if (cfl_container_adopt_variant(variant) != 0) { + if (variant_created) { + cfl_variant_destroy(variant); + } + + return -1; + } + if (o->variant != NULL && o->variant != variant) { cfl_variant_destroy(o->variant); } diff --git a/src/cfl_sds.c b/src/cfl_sds.c index 5422f69..dea6d61 100644 --- a/src/cfl_sds.c +++ b/src/cfl_sds.c @@ -224,7 +224,7 @@ cfl_sds_t cfl_sds_cat(cfl_sds_t s, const char *str, int len) (source_addr - buffer_addr) <= head->alloc) { source_offset = (size_t) (source_addr - buffer_addr); - if (append_len - 1 > head->alloc - source_offset) { + if (append_len - 1 >= head->alloc - source_offset) { return NULL; } diff --git a/src/cfl_variant.c b/src/cfl_variant.c index 44b3fa2..4143aa6 100644 --- a/src/cfl_variant.c +++ b/src/cfl_variant.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -337,6 +338,12 @@ struct cfl_variant *cfl_variant_create_from_array(struct cfl_array *value) instance = cfl_variant_create(); if (instance != NULL) { + if (value != NULL && + cfl_container_claim_array(value, instance) != 0) { + free(instance); + return NULL; + } + instance->data.as_array = value; instance->type = CFL_VARIANT_ARRAY; } @@ -350,6 +357,12 @@ struct cfl_variant *cfl_variant_create_from_kvlist(struct cfl_kvlist *value) instance = cfl_variant_create(); if (instance != NULL) { + if (value != NULL && + cfl_container_claim_kvlist(value, instance) != 0) { + free(instance); + return NULL; + } + instance->data.as_kvlist = value; instance->type = CFL_VARIANT_KVLIST; } @@ -390,6 +403,8 @@ void cfl_variant_destroy(struct cfl_variant *instance) return; } + cfl_container_release_variant(instance); + if (instance->type == CFL_VARIANT_STRING || instance->type == CFL_VARIANT_BYTES) { if (instance->data.as_string != NULL && !instance->referenced) { diff --git a/tests/array.c b/tests/array.c index 31d50b9..64f1c31 100644 --- a/tests/array.c +++ b/tests/array.c @@ -336,10 +336,10 @@ static void append_array_rejects_cycles() TEST_CHECK(ret == 0); ret = cfl_array_append_array(arr, child); - TEST_CHECK(ret == -1); + TEST_CHECK(ret < 0); ret = cfl_array_append_array(child, arr); - TEST_CHECK(ret == -1); + TEST_CHECK(ret < 0); cfl_array_destroy(arr); } @@ -362,26 +362,31 @@ static void append_variant_rejects_cycles() cfl_variant_destroy(variant); } -static void append_rejects_duplicate_variant() +static void append_rejects_shared_array_between_parents() { int ret; - struct cfl_array *arr; - struct cfl_variant *variant; + struct cfl_array *arr_a; + struct cfl_array *arr_b; + struct cfl_array *child; - arr = cfl_array_create(2); - TEST_CHECK(arr != NULL); + arr_a = cfl_array_create(1); + TEST_CHECK(arr_a != NULL); - variant = cfl_variant_create_from_string("value"); - TEST_CHECK(variant != NULL); + arr_b = cfl_array_create(1); + TEST_CHECK(arr_b != NULL); - ret = cfl_array_append(arr, variant); + child = cfl_array_create(0); + TEST_CHECK(child != NULL); + + ret = cfl_array_append_array(arr_a, child); TEST_CHECK(ret == 0); - ret = cfl_array_append(arr, variant); + ret = cfl_array_append_array(arr_b, child); TEST_CHECK(ret == -1); - TEST_CHECK(cfl_array_size(arr) == 1); + TEST_CHECK(cfl_array_size(arr_b) == 0); - cfl_array_destroy(arr); + cfl_array_destroy(arr_b); + cfl_array_destroy(arr_a); } static void append_kvlist_rejects_cycles() @@ -400,7 +405,7 @@ static void append_kvlist_rejects_cycles() TEST_CHECK(ret == 0); ret = cfl_kvlist_insert_array(kvlist, "cycle", arr); - TEST_CHECK(ret == -1); + TEST_CHECK(ret < 0); cfl_array_destroy(arr); } @@ -534,7 +539,7 @@ TEST_LIST = { {"append_kvlist", append_kvlist}, {"append_array_rejects_cycles", append_array_rejects_cycles}, {"append_variant_rejects_cycles", append_variant_rejects_cycles}, - {"append_rejects_duplicate_variant", append_rejects_duplicate_variant}, + {"append_rejects_shared_array_between_parents", append_rejects_shared_array_between_parents}, {"append_kvlist_rejects_cycles", append_kvlist_rejects_cycles}, {"remove_by_index", remove_by_index}, {"remove_by_reference", remove_by_reference}, diff --git a/tests/kvlist.c b/tests/kvlist.c index 20f1d0e..d813169 100644 --- a/tests/kvlist.c +++ b/tests/kvlist.c @@ -1238,6 +1238,10 @@ static void null_inputs() TEST_CHECK(variant == NULL); cfl_kvpair_destroy(NULL); + + variant = cfl_kvpair_take_value(NULL); + TEST_CHECK(variant == NULL); + cfl_kvlist_destroy(list); } @@ -1355,7 +1359,7 @@ static void reject_kvlist_cycles() TEST_CHECK(ret == -1); ret = cfl_kvlist_insert_kvlist(child, "parent", list); - TEST_CHECK(ret == -1); + TEST_CHECK(ret < 0); cfl_kvlist_destroy(list); } @@ -1378,26 +1382,31 @@ static void reject_variant_cycles() cfl_variant_destroy(variant); } -static void reject_duplicate_variant() +static void reject_shared_kvlist_between_parents() { int ret; - struct cfl_kvlist *list; - struct cfl_variant *variant; + struct cfl_kvlist *list_a; + struct cfl_kvlist *list_b; + struct cfl_kvlist *child; - list = cfl_kvlist_create(); - TEST_CHECK(list != NULL); + list_a = cfl_kvlist_create(); + TEST_CHECK(list_a != NULL); - variant = cfl_variant_create_from_string("value"); - TEST_CHECK(variant != NULL); + list_b = cfl_kvlist_create(); + TEST_CHECK(list_b != NULL); - ret = cfl_kvlist_insert(list, "one", variant); + child = cfl_kvlist_create(); + TEST_CHECK(child != NULL); + + ret = cfl_kvlist_insert_kvlist(list_a, "child", child); TEST_CHECK(ret == 0); - ret = cfl_kvlist_insert(list, "two", variant); + ret = cfl_kvlist_insert_kvlist(list_b, "child", child); TEST_CHECK(ret == -1); - TEST_CHECK(cfl_kvlist_count(list) == 1); + TEST_CHECK(cfl_kvlist_count(list_b) == 0); - cfl_kvlist_destroy(list); + cfl_kvlist_destroy(list_b); + cfl_kvlist_destroy(list_a); } static void reject_array_cycles() @@ -1416,11 +1425,90 @@ static void reject_array_cycles() TEST_CHECK(ret == 0); ret = cfl_array_append_kvlist(array, list); - TEST_CHECK(ret == -1); + TEST_CHECK(ret < 0); cfl_kvlist_destroy(list); } +static void move_taken_value_between_kvlists() +{ + int ret; + struct cfl_list *head; + struct cfl_kvlist *source; + struct cfl_kvlist *destination; + struct cfl_kvpair *pair; + struct cfl_variant *value; + struct cfl_variant *moved; + + source = cfl_kvlist_create(); + TEST_CHECK(source != NULL); + + destination = cfl_kvlist_create(); + TEST_CHECK(destination != NULL); + + ret = cfl_kvlist_insert_string(source, "source", "value"); + TEST_CHECK(ret == 0); + + head = source->list.next; + pair = cfl_list_entry(head, struct cfl_kvpair, _head); + value = cfl_kvpair_take_value(pair); + TEST_CHECK(value != NULL); + TEST_CHECK(pair->val == NULL); + + cfl_kvpair_destroy(pair); + TEST_CHECK(cfl_kvlist_count(source) == 0); + + ret = cfl_kvlist_insert(destination, "destination", value); + TEST_CHECK(ret == 0); + + moved = cfl_kvlist_fetch(destination, "destination"); + TEST_CHECK(moved == value); + TEST_CHECK(moved->type == CFL_VARIANT_STRING); + TEST_CHECK(strcmp(moved->data.as_string, "value") == 0); + + cfl_kvlist_destroy(source); + cfl_kvlist_destroy(destination); +} + +static void insert_then_detach_move_compatibility() +{ + int ret; + struct cfl_list *head; + struct cfl_kvlist *source; + struct cfl_kvlist *destination; + struct cfl_kvpair *pair; + struct cfl_variant *value; + struct cfl_variant *moved; + + source = cfl_kvlist_create(); + TEST_CHECK(source != NULL); + + destination = cfl_kvlist_create(); + TEST_CHECK(destination != NULL); + + ret = cfl_kvlist_insert_string(source, "source", "value"); + TEST_CHECK(ret == 0); + + head = source->list.next; + pair = cfl_list_entry(head, struct cfl_kvpair, _head); + value = pair->val; + + ret = cfl_kvlist_insert(destination, "destination", value); + TEST_CHECK(ret == 0); + + pair->val = NULL; + cfl_kvpair_destroy(pair); + TEST_CHECK(cfl_kvlist_count(source) == 0); + + moved = cfl_kvlist_fetch(destination, "destination"); + TEST_CHECK(moved == value); + TEST_CHECK(moved->type == CFL_VARIANT_STRING); + TEST_CHECK(strcmp(moved->data.as_string, "value") == 0); + + cfl_kvlist_destroy(source); + cfl_kvlist_destroy(destination); +} + TEST_LIST = { {"create_destroy", create_destroy}, {"count", count}, @@ -1454,7 +1542,9 @@ TEST_LIST = { {"print_write_error", print_write_error}, {"reject_kvlist_cycles", reject_kvlist_cycles}, {"reject_variant_cycles", reject_variant_cycles}, - {"reject_duplicate_variant", reject_duplicate_variant}, + {"reject_shared_kvlist_between_parents", reject_shared_kvlist_between_parents}, {"reject_array_cycles", reject_array_cycles}, + {"move_taken_value_between_kvlists", move_taken_value_between_kvlists}, + {"insert_then_detach_move_compatibility", insert_then_detach_move_compatibility}, { 0 } }; diff --git a/tests/object.c b/tests/object.c index 7b5f2b5..872cee3 100644 --- a/tests/object.c +++ b/tests/object.c @@ -305,6 +305,32 @@ static void test_reject_nested_variant_reuse() cfl_object_destroy(object); } +static void test_reject_shared_kvlist_between_objects() +{ + int ret; + struct cfl_object *object_a; + struct cfl_object *object_b; + struct cfl_kvlist *list; + + object_a = cfl_object_create(); + TEST_CHECK(object_a != NULL); + + object_b = cfl_object_create(); + TEST_CHECK(object_b != NULL); + + list = cfl_kvlist_create(); + TEST_CHECK(list != NULL); + + ret = cfl_object_set(object_a, CFL_OBJECT_KVLIST, list); + TEST_CHECK(ret == 0); + + ret = cfl_object_set(object_b, CFL_OBJECT_KVLIST, list); + TEST_CHECK(ret == -1); + + cfl_object_destroy(object_b); + cfl_object_destroy(object_a); +} + TEST_LIST = { { "test_basics", test_basics }, { "test_replace_and_print", test_replace_and_print }, @@ -313,5 +339,6 @@ TEST_LIST = { { "test_reject_nested_kvlist_reuse", test_reject_nested_kvlist_reuse }, { "test_reject_nested_array_reuse", test_reject_nested_array_reuse }, { "test_reject_nested_variant_reuse", test_reject_nested_variant_reuse }, + { "test_reject_shared_kvlist_between_objects", test_reject_shared_kvlist_between_objects }, { 0 } }; diff --git a/tests/sds.c b/tests/sds.c index d54b11a..e8baf25 100644 --- a/tests/sds.c +++ b/tests/sds.c @@ -123,11 +123,60 @@ static void test_sds_rejects_oversized_in_buffer_slice() cfl_sds_destroy(s); } +static void test_sds_in_buffer_slice_boundaries() +{ + cfl_sds_t s; + cfl_sds_t tmp; + + s = cfl_sds_create("abcdef"); + TEST_CHECK(s != NULL); + if (s == NULL) { + return; + } + + tmp = cfl_sds_cat(s, s + 4, 2); + TEST_CHECK(tmp != NULL); + if (tmp == NULL) { + cfl_sds_destroy(s); + return; + } + s = tmp; + + TEST_CHECK(cfl_sds_len(s) == 8); + TEST_CHECK(strcmp("abcdefef", s) == 0); + cfl_sds_destroy(s); + + s = cfl_sds_create("abcdef"); + TEST_CHECK(s != NULL); + if (s == NULL) { + return; + } + + tmp = cfl_sds_cat(s, s + 5, 2); + TEST_CHECK(tmp == NULL); + TEST_CHECK(cfl_sds_len(s) == 6); + TEST_CHECK(strcmp("abcdef", s) == 0); + cfl_sds_destroy(s); + + s = cfl_sds_create("abcdef"); + TEST_CHECK(s != NULL); + if (s == NULL) { + return; + } + + tmp = cfl_sds_cat(s, s + cfl_sds_alloc(s), 1); + TEST_CHECK(tmp == NULL); + TEST_CHECK(cfl_sds_len(s) == 6); + TEST_CHECK(strcmp("abcdef", s) == 0); + cfl_sds_destroy(s); +} + TEST_LIST = { { "sds_usage" , test_sds_usage}, { "sds_printf", test_sds_printf}, { "sds_invalid_inputs", test_sds_invalid_inputs}, { "sds_self_append", test_sds_self_append}, { "sds_rejects_oversized_in_buffer_slice", test_sds_rejects_oversized_in_buffer_slice}, + { "sds_in_buffer_slice_boundaries", test_sds_in_buffer_slice_boundaries}, { 0 } };