From eaa6b723bcb9c3569e47497d2c1d95a4f42e935b Mon Sep 17 00:00:00 2001 From: Laria Carolin Chabowski Date: Tue, 4 Jan 2022 21:22:46 +0100 Subject: [PATCH] Fix refcounted strings Increasing the refcount (confusingly called copy before, fixed that too) didn't work properly, as the object was copied and the refcount was only updated in one of the copies. --- src/apfl.h | 33 ++++++++++++++++++--------------- src/eval.c | 2 +- src/internal.c | 2 +- src/strings.c | 39 ++++++++++++++++++++++----------------- src/value.c | 7 +++---- 5 files changed, 45 insertions(+), 38 deletions(-) diff --git a/src/apfl.h b/src/apfl.h index 18ff5fb..e271b74 100644 --- a/src/apfl.h +++ b/src/apfl.h @@ -30,10 +30,9 @@ struct apfl_string { size_t len; }; -struct apfl_refcounted_string { - unsigned refcount; - struct apfl_string string; -}; +struct apfl_refcounted_string_data; + +typedef struct apfl_refcounted_string_data *apfl_refcounted_string; #define APFL_STR_FMT "%.*s" #define APFL_STR_FMT_ARGS(s) (int)(s).len,(s).bytes @@ -42,14 +41,14 @@ struct apfl_string_view apfl_string_view_from_view(struct apfl_string_view); struct apfl_string_view apfl_string_view_from_cstr(char *); struct apfl_string_view apfl_string_view_from_const_cstr(const char *); struct apfl_string_view apfl_string_view_from_string(struct apfl_string); -struct apfl_string_view apfl_string_view_from_refcounted_string(struct apfl_refcounted_string *); +struct apfl_string_view apfl_string_view_from_refcounted_string(apfl_refcounted_string); -#define apfl_string_view_from(s) _Generic((s), \ - struct apfl_string: apfl_string_view_from_string, \ - struct apfl_string_view: apfl_string_view_from_view, \ - struct apfl_refcounted_string *: apfl_string_view_from_refcounted_string, \ - char *: apfl_string_view_from_cstr, \ - const char *: apfl_string_view_from_const_cstr \ +#define apfl_string_view_from(s) _Generic((s), \ + struct apfl_string: apfl_string_view_from_string, \ + struct apfl_string_view: apfl_string_view_from_view, \ + apfl_refcounted_string: apfl_string_view_from_refcounted_string, \ + char *: apfl_string_view_from_cstr, \ + const char *: apfl_string_view_from_const_cstr \ )(s) int apfl_string_view_cmp(struct apfl_string_view, struct apfl_string_view); @@ -82,13 +81,17 @@ struct apfl_string apfl_string_builder_move_string(struct apfl_string_builder *) #define apfl_string_builder_append_cstr(builder, cstr) (apfl_string_builder_append((builder), apfl_string_view_from_cstr((cstr)))) -struct apfl_refcounted_string *apfl_string_move_into_new_refcounted(struct apfl_string *); -struct apfl_refcounted_string *apfl_refcounted_string_copy(struct apfl_refcounted_string *); +apfl_refcounted_string apfl_string_move_into_new_refcounted(struct apfl_string *); + +/* Increases the reference of the refcounted string. + * Returns the same apfl_refcounted_string value. + */ +apfl_refcounted_string apfl_refcounted_string_incref(apfl_refcounted_string); /* Unrefs the refcounted string. It is no longer allowed to use the pointer * after calling this function with it! */ -void apfl_refcounted_string_unref(struct apfl_refcounted_string *); +void apfl_refcounted_string_unref(apfl_refcounted_string ); // Tokens @@ -518,7 +521,7 @@ struct apfl_value { union { bool boolean; apfl_number number; - struct apfl_refcounted_string *string; + apfl_refcounted_string string; struct apfl_list *list; }; }; diff --git a/src/eval.c b/src/eval.c index eac8b8c..b53e21d 100644 --- a/src/eval.c +++ b/src/eval.c @@ -36,7 +36,7 @@ fatal(void) static struct apfl_result evaluate_constant(struct apfl_expr_const *constant) { - struct apfl_refcounted_string *rcstring; + apfl_refcounted_string rcstring; switch (constant->type) { case APFL_EXPR_CONST_NIL: diff --git a/src/internal.c b/src/internal.c index 67d78db..3630df7 100644 --- a/src/internal.c +++ b/src/internal.c @@ -19,7 +19,7 @@ apfl_print_indented(unsigned indent, FILE *f, const char* fmt, ...) bool apfl_refcount_dec(unsigned *rc) { - if (rc == 0) { + if (rc == NULL) { return false; } return --(*rc) == 0; diff --git a/src/strings.c b/src/strings.c index 44e93dd..ae7bb80 100644 --- a/src/strings.c +++ b/src/strings.c @@ -8,6 +8,11 @@ #include "internal.h" #include "resizable.h" +struct apfl_refcounted_string_data { + unsigned refcount; + struct apfl_string string; +}; + struct apfl_string_view apfl_string_view_from_view(struct apfl_string_view view) { @@ -142,21 +147,25 @@ apfl_string_builder_move_string(struct apfl_string_builder *builder) return str; } -struct -apfl_string_view apfl_string_view_from_refcounted_string(struct apfl_refcounted_string *rcstring) +struct apfl_string_view +apfl_string_view_from_refcounted_string(apfl_refcounted_string rcstring) { + if (rcstring == NULL) { + return apfl_string_view_from(apfl_string_blank()); + } + return apfl_string_view_from(rcstring->string); } -struct apfl_refcounted_string * +apfl_refcounted_string apfl_string_move_into_new_refcounted(struct apfl_string *src) { - struct apfl_refcounted_string *dst = ALLOC(struct apfl_refcounted_string); + apfl_refcounted_string dst = ALLOC(struct apfl_refcounted_string_data); if (dst == NULL) { return NULL; } - *dst = (struct apfl_refcounted_string) { + *dst = (struct apfl_refcounted_string_data) { .refcount = 1, .string = apfl_string_move(src), }; @@ -164,24 +173,20 @@ apfl_string_move_into_new_refcounted(struct apfl_string *src) return dst; } -struct apfl_refcounted_string * -apfl_refcounted_string_copy(struct apfl_refcounted_string *src) +apfl_refcounted_string +apfl_refcounted_string_incref(apfl_refcounted_string rcstring) { - struct apfl_refcounted_string *dst = ALLOC(struct apfl_refcounted_string); - if (dst == NULL) { - return NULL; + if (rcstring != NULL) { + rcstring->refcount++; } - - *dst = *src; - dst->refcount++; - - return dst; + return rcstring; } void -apfl_refcounted_string_unref(struct apfl_refcounted_string *rcstring) +apfl_refcounted_string_unref(apfl_refcounted_string rcstring) { - if (apfl_refcount_dec(&rcstring->refcount)) { + if (rcstring != NULL && apfl_refcount_dec(&rcstring->refcount)) { apfl_string_deinit(&rcstring->string); + free(rcstring); } } diff --git a/src/value.c b/src/value.c index b3bb24d..1c94784 100644 --- a/src/value.c +++ b/src/value.c @@ -50,9 +50,7 @@ apfl_value_copy(struct apfl_value *dst, struct apfl_value src) // Nothing to do goto ok; case APFL_VALUE_STRING: - if ((dst->string = apfl_refcounted_string_copy(src.string)) == NULL) { - return false; - } + dst->string = apfl_refcounted_string_incref(src.string); goto ok; case APFL_VALUE_LIST: assert(src.list != NULL); @@ -108,7 +106,8 @@ apfl_value_deinit(struct apfl_value *value) case APFL_VALUE_NUMBER: goto ok; case APFL_VALUE_STRING: - DESTROY(value->string, apfl_refcounted_string_unref); + apfl_refcounted_string_unref(value->string); + value->string = NULL; goto ok; case APFL_VALUE_LIST: if (apfl_refcount_dec(&value->list->refcount)) {