From 96b1fecdcd699cda4ead24d087dd9fe16b80c8bd Mon Sep 17 00:00:00 2001 From: Laria Carolin Chabowski Date: Wed, 5 Jan 2022 21:54:37 +0100 Subject: [PATCH] Hashmap: Make copy callbacks return void Since we're using refcounts, we don't really copy anything and no error can occur. So let's make these callbacks return void to simplify things. This also makes the return value false of the value getters unambiguous: It now always means that the key was not present. --- src/hashmap.c | 50 ++++++++++++++++++-------------------------------- src/hashmap.h | 8 ++++---- src/value.c | 29 +++++------------------------ 3 files changed, 27 insertions(+), 60 deletions(-) diff --git a/src/hashmap.c b/src/hashmap.c index c3de2a6..837b21a 100644 --- a/src/hashmap.c +++ b/src/hashmap.c @@ -84,25 +84,23 @@ destroy_value(apfl_hashmap map, void *value) } } -static bool +static void copy_key(apfl_hashmap map, void *dest, void *src) { if (HAS_CALLBACK(map, copy_key)) { - return INVOKE_CALLBACK(map, copy_key, dest, src); + INVOKE_CALLBACK(map, copy_key, dest, src); } else { memcpy(dest, src, map->keysize); - return true; } } -static bool +static void copy_value(apfl_hashmap map, void *dest, void *src) { if (HAS_CALLBACK(map, copy_value)) { - return INVOKE_CALLBACK(map, copy_value, dest, src); + INVOKE_CALLBACK(map, copy_value, dest, src); } else { memcpy(dest, src, map->valsize); - return true; } } @@ -143,7 +141,8 @@ set_in_bucket(apfl_hashmap map, struct bucket *bucket, void *key, void *value) void *dest = KVADDR(bucket->values, valsize, i); destroy_value(map, dest); - return copy_value(map, dest, value); + copy_value(map, dest, value); + return true; } if (bucket->len <= bucket->cap) { @@ -166,14 +165,8 @@ set_in_bucket(apfl_hashmap map, struct bucket *bucket, void *key, void *value) bucket->cap = new_cap; } - if (!copy_key(map, KVADDR(bucket->keys, keysize, bucket->len), key)) { - return false; - } - - if (!copy_value(map, KVADDR(bucket->values, valsize, bucket->len), value)) { - destroy_key(map, KVADDR(bucket->keys, keysize, bucket->len)); - return false; - } + copy_key(map, KVADDR(bucket->keys, keysize, bucket->len), key); + copy_value(map, KVADDR(bucket->values, valsize, bucket->len), value); bucket->len++; @@ -190,9 +183,7 @@ get_in_bucket(const apfl_hashmap map, struct bucket *bucket, const void *key, vo if (value != NULL) { size_t valsize = map->valsize; - if (!copy_value(map, value, KVADDR(bucket->values, valsize, i))) { - return false; // TODO: This way, we cant distinguish an error in copy_value from a non-set key - } + copy_value(map, value, KVADDR(bucket->values, valsize, i)); } return true; @@ -366,22 +357,17 @@ apfl_hashmap_copy(apfl_hashmap src) for (; dstbucket->len < len; dstbucket->len++) { void *keyaddr = KVADDR(dstbucket->keys, keysize, dstbucket->len); - if (!copy_key( + copy_key( dst, keyaddr, KVADDR(srcbucket->keys, keysize, srcbucket->len) - )) { - goto fail; - } + ); - if (!copy_value( + copy_value( dst, KVADDR(dstbucket->values, valsize, dstbucket->len), KVADDR(srcbucket->values, valsize, srcbucket->len) - )) { - destroy_key(dst, keyaddr); - goto fail; - } + ); } } @@ -451,28 +437,28 @@ apfl_hashmap_cursor_next(apfl_hashmap_cursor cursor) struct bucket *bucket = cursor_get_bucket(cursor); \ \ if (bucket == NULL) { \ - return false; /* End already reached */ \ + return; /* End already reached */ \ } \ \ if (cursor->i >= bucket->len) { \ - return false; \ + return; \ } \ \ size_t size = cursor->map->sizememb; \ \ - return copy( \ + copy( \ cursor->map, \ out, \ KVADDR(bucket->bucketmemb, size, cursor->i) \ ); \ -bool +void apfl_hashmap_cursor_get_key(apfl_hashmap_cursor cursor, void *key) { CURSOR_GET(cursor, key, keys, keysize, copy_key) } -bool +void apfl_hashmap_cursor_get_value(apfl_hashmap_cursor cursor, void *value) { CURSOR_GET(cursor, value, values, valsize, copy_value) diff --git a/src/hashmap.h b/src/hashmap.h index 078f5b1..17c898c 100644 --- a/src/hashmap.h +++ b/src/hashmap.h @@ -36,11 +36,11 @@ struct apfl_hashmap_callbacks { // Copies a key. Returns true on success, false on failure. // If not provided, the bytes will be copied with memcpy. - bool (*copy_key) (void *opaque, void *dest, void *src); + void (*copy_key) (void *opaque, void *dest, void *src); // Copies a value. Returns true on success, false on failure. // If not provided, the bytes will be copied with memcpy. - bool (*copy_value) (void *opaque, void *dest, void *src); + void (*copy_value) (void *opaque, void *dest, void *src); }; apfl_hash apfl_hash_fnv1a_add(const void *, size_t len, apfl_hash); @@ -60,8 +60,8 @@ apfl_hashmap apfl_hashmap_copy(apfl_hashmap src); apfl_hashmap_cursor apfl_hashmap_get_cursor(apfl_hashmap); bool apfl_hashmap_cursor_is_end(apfl_hashmap_cursor); void apfl_hashmap_cursor_next(apfl_hashmap_cursor); -bool apfl_hashmap_cursor_get_key(apfl_hashmap_cursor, void *key); -bool apfl_hashmap_cursor_get_value(apfl_hashmap_cursor, void *value); +void apfl_hashmap_cursor_get_key(apfl_hashmap_cursor, void *key); +void apfl_hashmap_cursor_get_value(apfl_hashmap_cursor, void *value); #define apfl_hashmap_cursor_destroy(cur) (free(cur)) diff --git a/src/value.c b/src/value.c index bba8eaa..9f6704e 100644 --- a/src/value.c +++ b/src/value.c @@ -64,7 +64,7 @@ dict_destroy_key_or_value(void *opaque, void *kv) apfl_value_deinit(kv); } -static bool +static void dict_copy_key_or_value(void *opaque, void *_dest, void *_src) { (void)opaque; @@ -73,8 +73,6 @@ dict_copy_key_or_value(void *opaque, void *_dest, void *_src) struct apfl_value *src = _src; *dest = apfl_value_incref(*src); - - return true; } static const struct apfl_hashmap_callbacks dict_hashmap_callbacks = { @@ -164,15 +162,8 @@ print(unsigned indent, FILE *out, struct apfl_value value, bool skip_first_inden apfl_print_indented(first_indent, out, "[\n"); for (; !apfl_hashmap_cursor_is_end(cur); apfl_hashmap_cursor_next(cur)) { struct apfl_value k, v; - if (!apfl_hashmap_cursor_get_key(cur, &k)) { - apfl_hashmap_cursor_destroy(cur); - return; // TODO: Handle failure in a better way - } - if (!apfl_hashmap_cursor_get_value(cur, &v)) { - apfl_hashmap_cursor_destroy(cur); - apfl_value_deinit(&k); - return; // TODO: Handle failure in a better way - } + apfl_hashmap_cursor_get_key(cur, &k); + apfl_hashmap_cursor_get_value(cur, &v); print(indent+1, out, k, false); apfl_value_deinit(&k); @@ -256,18 +247,8 @@ dict_eq_inner(apfl_hashmap_cursor cur, const apfl_dict b) struct apfl_value key; struct apfl_value val; - // TODO: It's rather ugly that we simply return false when getting key/value fails. - - if (!apfl_hashmap_cursor_get_key(cur, &key)) { - assert(false); - return false; - } - - if (!apfl_hashmap_cursor_get_value(cur, &val)) { - apfl_value_deinit(&key); - assert(false); - return false; - } + apfl_hashmap_cursor_get_key(cur, &key); + apfl_hashmap_cursor_get_value(cur, &val); struct apfl_value other_val; if (!apfl_hashmap_get(b->map, &key, &other_val)) {