Mercurial > lbo > hg > ylisp
changeset 97:3b468dad363f
value: Fix some issues with regards to yref_t ownership and nested destroying
author | Lewin Bormann <lbo@spheniscida.de> |
---|---|
date | Mon, 26 Aug 2019 17:01:14 +0200 |
parents | f2b65c8f6a24 |
children | 8498c1faba07 |
files | src/value.c src/value.h src/value_test.c |
diffstat | 3 files changed, 65 insertions(+), 9 deletions(-) [+] |
line wrap: on
line diff
--- a/src/value.c Mon Aug 26 17:00:47 2019 +0200 +++ b/src/value.c Mon Aug 26 17:01:14 2019 +0200 @@ -46,7 +46,7 @@ } bool yvalue_is_valid(yvalue_id_t id) { - return (id & YVALUE_ID_MARKER) == YVALUE_ID_MARKER; + return (id & YVALUE_ID_MARKER) == YVALUE_ID_MARKER && id < yvalue_counter; } YREF_TYPE yref_type(yref_t* ref) { @@ -79,6 +79,19 @@ return ref; } +yref_t yref_clone(yref_t* ref) { + switch (yref_type(ref)) { + case YREF_SYM: + // Copy string. + return yref_new_c(ystr_str(&ref->ref.sym)); + case YREF_ID: + return *ref; + default: + assert(yref_type(ref) == YREF_SYM || yref_type(ref) == YREF_ID); + return yref_new(); + } +} + yref_t yvalue_clone(yref_t* ref) { yvalue_t* orig = yvalue_get(*ref); assert(orig->typ == YVALUE_EXPR || orig->typ == YVALUE_FUNC); @@ -86,7 +99,8 @@ // Functions are immutable for now. return *ref; } else if (orig->typ == YVALUE_EXPR) { - yvalue_t new = { .typ = YVALUE_EXPR, .value.expr = yexpr_copy(&orig->value.expr) }; + yvalue_t new = {.typ = YVALUE_EXPR, + .value.expr = yexpr_copy(&orig->value.expr)}; return yvalue_set(YVALUE_INSERT, &new); } return yref_new_id(YVALUE_INVALID_ID); @@ -171,6 +185,8 @@ if (typ == YREF_ID) return; if (typ == YREF_SYM) { yvalue_id_t id = yvalue_resolve_or_create_symbol(&ref->ref.sym); + // CAREFUL: This may cause double-frees if `ref` is copied from another + // ref. ystr_destroy(&ref->ref.sym); ref->ref.id = id; } @@ -183,13 +199,20 @@ // ref == YVALUE_INSERT if (typ == YREF_ID && ref.ref.id == YVALUE_INSERT.ref.id) { // Unnamed value. - ref.ref.id = yvalue_counter++; - size_t new_index = YVEC_PUSH(&yvalue_table, val); - assert(new_index == (yvalue_counter - YVALUE_COUNTER_OFFSET - 1)); + ref = yref_new(); + yvalue_t* valp = yvalue_get(ref); + assert(valp != NULL); + *valp = *val; return ref; } else { - yvalue_resolve_or_create_ref(&ref); - yvalue_t* valp = yvalue_get(ref); + // Resolve reference. + yvalue_id_t id; + if (typ == YREF_ID) + id = ref.ref.id; + else + id = yvalue_resolve_or_create_symbol(&ref.ref.sym); + assert(yvalue_is_valid(id)); + yvalue_t* valp = yvalue_get(yref_new_id(id)); assert(valp != NULL); *valp = *val; return ref;
--- a/src/value.h Mon Aug 26 17:00:47 2019 +0200 +++ b/src/value.h Mon Aug 26 17:01:14 2019 +0200 @@ -46,6 +46,12 @@ YREF_TYPE yref_type(yref_t* ref); /** + * Create a new reference pointing to the same value as `ref`. The caller + * assumes ownership of the returned reference and needs to destroy it later. + */ +yref_t yref_clone(yref_t* ref); + +/** * Create a new unnamed reference. */ yref_t yref_new(void); @@ -126,6 +132,10 @@ /** * @brief Translate a symbol in `ref` to an ID, or create a new ID. `ref` is * updated to be an ID ref. Caller retains ownership of `ref`. + * + * NOTE: If `ref` is symbolic, this function frees the symbol string. This + * means that it is an error to supply a pointer to a reference that is copied + * from another reference that is freed later (double-free). */ void yvalue_resolve_or_create_ref(yref_t* ref); @@ -136,7 +146,8 @@ * deallocated though). If the reference is a symbolic reference that does not * exist yet, it is resolved and created first. * - * The reference at which the value was insertes is returned. + * The reference at which the value was inserted at is returned (this is only + * useful if YVALUE_INSERT is used). */ yref_t yvalue_set(yref_t ref, yvalue_t* val);
--- a/src/value_test.c Mon Aug 26 17:00:47 2019 +0200 +++ b/src/value_test.c Mon Aug 26 17:01:14 2019 +0200 @@ -20,18 +20,39 @@ } void test_ref_create_resolve(void) { - ystr_t symbol = ystr_new("rcr_symbol"); + ystr_t symbol = ystr_new("rcr_symbol_long_long"); ystr_t symbol2 = ystr_new(NULL); ystr_copy(&symbol, &symbol2); yref_t ref = yref_new_str(&symbol); // symbol is now owned by ref. + yref_t cloned = yref_clone(&ref); + assert(cloned.ref.sym.inner.big.base != ref.ref.sym.inner.big.base); + yvalue_resolve_or_create_ref(&ref); assert(YVALUE_INVALID_ID != yvalue_resolve_symbol(&symbol2)); assert(YREF_ID == yref_type(&ref)); + assert(ref.ref.id == yref_clone(&ref).ref.id); + assert(YREF_SYM == yref_type(&cloned)); assert(ref.ref.id == yvalue_resolve_symbol(&symbol2)); ystr_destroy(&symbol2); yref_destroy(&ref); + yref_destroy(&cloned); +} + +void test_value_clone(void) { + yvalue_t first_value = { .typ = YVALUE_EXPR, .value.expr = yexpr_new() }; + yexpr_set_str(&first_value.value.expr, ystr_new("a long external test string")); + yref_t first = yvalue_set(YVALUE_INSERT, &first_value); + yref_t second = yvalue_set(YVALUE_INSERT, &first_value); + yref_t cloned = yvalue_clone(&first); + assert(first.ref.id != cloned.ref.id); + + *ystr_at(&yvalue_get(first)->value.expr.value.str, 0) = 'A'; + assert('A' == *ystr_at(&yvalue_get(first)->value.expr.value.str, 0)); + assert('A' == *ystr_at(&yvalue_get(second)->value.expr.value.str, 0)); + assert('A' == *ystr_at(&yvalue_get(yref_clone(&first))->value.expr.value.str, 0)); + assert('a' == *ystr_at(&yvalue_get(cloned)->value.expr.value.str, 0)); } void test_value_set(void) { @@ -83,5 +104,6 @@ int main(void) { test_value_create_resolve(); test_ref_create_resolve(); + test_value_clone(); test_value_set(); }