From 9bcdeff95e10ae2bb1c915f10686bd8c6c6659be Mon Sep 17 00:00:00 2001 From: =?utf8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= Date: Sat, 22 Aug 2020 10:24:35 +0200 Subject: [PATCH] lib/cache: type safety of the cache API pointers See the definition of kr_cdb_pt for details. --- daemon/bindings/cache.c | 2 +- daemon/lua/kres-gen.lua | 4 +-- daemon/lua/kres-gen.sh | 2 +- lib/cache/api.h | 2 +- lib/cache/cdb_api.h | 30 +++++++++------- lib/cache/cdb_lmdb.c | 80 +++++++++++++++++++++++------------------ lib/cache/cdb_lmdb.h | 4 ++- utils/cache_gc/db.c | 4 +-- 8 files changed, 74 insertions(+), 54 deletions(-) diff --git a/daemon/bindings/cache.c b/daemon/bindings/cache.c index 49cb978a2..56b75fc93 100644 --- a/daemon/bindings/cache.c +++ b/daemon/bindings/cache.c @@ -95,7 +95,7 @@ static int cache_stats(lua_State *L) add_stat(read_leq); add_stat(read_leq_miss); /* usage_percent statistics special case - double */ - struct libknot_lmdb_env *libknot_db = knot_db_t_kres2libknot(cache->db); + struct libknot_lmdb_env *libknot_db = kr_cdb_pt2knot_db_t(cache->db); cache->stats.usage_percent = cache->api->usage_percent(libknot_db); lua_pushnumber(L, cache->stats.usage_percent); lua_setfield(L, -2, "usage_percent"); diff --git a/daemon/lua/kres-gen.lua b/daemon/lua/kres-gen.lua index 7ba576f15..eef26bfd7 100644 --- a/daemon/lua/kres-gen.lua +++ b/daemon/lua/kres-gen.lua @@ -5,7 +5,6 @@ local ffi = require('ffi') typedef struct knot_dump_style knot_dump_style_t; extern const knot_dump_style_t KNOT_DUMP_STYLE_DEFAULT; -typedef void knot_db_t; struct kr_cdb_api {}; struct lru {}; @@ -195,6 +194,7 @@ struct kr_request { unsigned int count_fail_row; }; enum kr_rank {KR_RANK_INITIAL, KR_RANK_OMIT, KR_RANK_TRY, KR_RANK_INDET = 4, KR_RANK_BOGUS, KR_RANK_MISMATCH, KR_RANK_MISSING, KR_RANK_INSECURE, KR_RANK_AUTH = 16, KR_RANK_SECURE = 32}; +typedef struct kr_cdb * kr_cdb_pt; struct kr_cdb_stats { uint64_t open; uint64_t close; @@ -215,7 +215,7 @@ struct kr_cdb_stats { }; typedef struct uv_timer_s uv_timer_t; struct kr_cache { - knot_db_t *db; + kr_cdb_pt db; const struct kr_cdb_api *api; struct kr_cdb_stats stats; uint32_t ttl_min; diff --git a/daemon/lua/kres-gen.sh b/daemon/lua/kres-gen.sh index 130703352..bd5c1fd46 100755 --- a/daemon/lua/kres-gen.sh +++ b/daemon/lua/kres-gen.sh @@ -56,7 +56,6 @@ printf -- "--[[ This file is generated by ./kres-gen.sh ]] ffi.cdef[[\n" printf " typedef struct knot_dump_style knot_dump_style_t; extern const knot_dump_style_t KNOT_DUMP_STYLE_DEFAULT; -typedef void knot_db_t; struct kr_cdb_api {}; struct lru {}; " @@ -114,6 +113,7 @@ ${CDEFS} ${LIBKRES} types <<-EOF struct kr_request_qsource_flags struct kr_request enum kr_rank + typedef kr_cdb_pt struct kr_cdb_stats typedef uv_timer_t struct kr_cache diff --git a/lib/cache/api.h b/lib/cache/api.h index e0786638d..28b3eb6ef 100644 --- a/lib/cache/api.h +++ b/lib/cache/api.h @@ -26,7 +26,7 @@ int cache_stash(kr_layer_t *ctx, knot_pkt_t *pkt); */ struct kr_cache { - knot_db_t *db; /**< Storage instance */ + kr_cdb_pt db; /**< Storage instance */ const struct kr_cdb_api *api; /**< Storage engine */ struct kr_cdb_stats stats; uint32_t ttl_min, ttl_max; /**< TTL limits */ diff --git a/lib/cache/cdb_api.h b/lib/cache/cdb_api.h index 546964fde..33ec2ffa2 100644 --- a/lib/cache/cdb_api.h +++ b/lib/cache/cdb_api.h @@ -33,6 +33,12 @@ struct kr_cdb_stats { double usage_percent; }; +/*! Pointer to a cache structure. + * + * This struct is opaque and never defined; the purpose is to get better + * type safety than with void *. + */ +typedef struct kr_cdb *kr_cdb_pt; /*! Cache database API. * This is a simplified version of generic DB API from libknot, @@ -43,48 +49,48 @@ struct kr_cdb_api { /* Context operations */ - int (*open)(knot_db_t **db, struct kr_cdb_stats *stat, struct kr_cdb_opts *opts, knot_mm_t *mm); - void (*close)(knot_db_t *db, struct kr_cdb_stats *stat); - int (*count)(knot_db_t *db, struct kr_cdb_stats *stat); - int (*clear)(knot_db_t *db, struct kr_cdb_stats *stat); + int (*open)(kr_cdb_pt *db, struct kr_cdb_stats *stat, struct kr_cdb_opts *opts, knot_mm_t *mm); + void (*close)(kr_cdb_pt db, struct kr_cdb_stats *stat); + int (*count)(kr_cdb_pt db, struct kr_cdb_stats *stat); + int (*clear)(kr_cdb_pt db, struct kr_cdb_stats *stat); /** Run after a row of operations to release transaction/lock if needed. */ - int (*commit)(knot_db_t *db, struct kr_cdb_stats *stat); + int (*commit)(kr_cdb_pt db, struct kr_cdb_stats *stat); /* Data access */ - int (*read)(knot_db_t *db, struct kr_cdb_stats *stat, + int (*read)(kr_cdb_pt db, struct kr_cdb_stats *stat, const knot_db_val_t *key, knot_db_val_t *val, int maxcount); - int (*write)(knot_db_t *db, struct kr_cdb_stats *stat, const knot_db_val_t *key, + int (*write)(kr_cdb_pt db, struct kr_cdb_stats *stat, const knot_db_val_t *key, knot_db_val_t *val, int maxcount); /** Remove maxcount keys. * \returns the number of succesfully removed keys or the first error code * It returns on first error, but ENOENT is not considered an error. */ - int (*remove)(knot_db_t *db, struct kr_cdb_stats *stat, + int (*remove)(kr_cdb_pt db, struct kr_cdb_stats *stat, knot_db_val_t keys[], int maxcount); /* Specialised operations */ /** Find key-value pairs that are prefixed by the given key, limited by maxcount. * \return the number of pairs or negative error. */ - int (*match)(knot_db_t *db, struct kr_cdb_stats *stat, + int (*match)(kr_cdb_pt db, struct kr_cdb_stats *stat, knot_db_val_t *key, knot_db_val_t keyval[][2], int maxcount); /** Less-or-equal search (lexicographic ordering). * On successful return, key->data and val->data point to DB-owned data. * return: 0 for equality, > 0 for less, < 0 kr_error */ - int (*read_leq)(knot_db_t *db, struct kr_cdb_stats *stat, + int (*read_leq)(kr_cdb_pt db, struct kr_cdb_stats *stat, knot_db_val_t *key, knot_db_val_t *val); double (*usage_percent)(knot_db_t *db); /** Return the current cache size limit in bytes; could be cached by check_health(). */ - size_t (*get_maxsize)(knot_db_t *db); + size_t (*get_maxsize)(kr_cdb_pt db); /** Perform maintenance. * In LMDB case it checks whether data.mdb is still the same * and reopens it if it isn't; it errors out if the file doesn't exist anymore. * \return 0 if OK, 1 if reopened OK, < 0 kr_error */ - int (*check_health)(knot_db_t *db, struct kr_cdb_stats *stat); + int (*check_health)(kr_cdb_pt db, struct kr_cdb_stats *stat); }; diff --git a/lib/cache/cdb_lmdb.c b/lib/cache/cdb_lmdb.c index e458e4968..ff152fb7a 100644 --- a/lib/cache/cdb_lmdb.c +++ b/lib/cache/cdb_lmdb.c @@ -59,8 +59,21 @@ struct libknot_lmdb_env { knot_mm_t *pool; }; +/** Type-safe conversion helper. + * + * We keep lmdb_env as a separate type from kr_db_pt, as different implementation of API + * would need to define the contents differently. + */ +static inline struct lmdb_env * db2env(kr_cdb_pt db) +{ + return (struct lmdb_env *)db; +} +static inline kr_cdb_pt env2db(struct lmdb_env *env) +{ + return (kr_cdb_pt)env; +} -static int cdb_commit(knot_db_t *db, struct kr_cdb_stats *stats); +static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats); /** @brief Convert LMDB error code. */ static int lmdb_error(int error) @@ -94,7 +107,7 @@ static inline MDB_val val_knot2mdb(knot_db_val_t v) * It's much lighter than reopen_env(). */ static int refresh_mapsize(struct lmdb_env *env) { - int ret = cdb_commit(env, NULL); + int ret = cdb_commit(env2db(env), NULL); if (!ret) ret = lmdb_error(mdb_env_set_mapsize(env->env, 0)); if (ret) return ret; @@ -196,9 +209,9 @@ static int txn_get(struct lmdb_env *env, MDB_txn **txn, bool rdonly) return kr_ok(); } -static int cdb_commit(knot_db_t *db, struct kr_cdb_stats *stats) +static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); int ret = kr_ok(); if (env->txn.rw) { if (stats) stats->commit++; @@ -221,7 +234,7 @@ static int txn_curs_get(struct lmdb_env *env, MDB_cursor **curs, struct kr_cdb_s } /* Only in a read-only txn; TODO: it's a bit messy/coupled */ if (env->txn.rw) { - int ret = cdb_commit(env, stats); + int ret = cdb_commit(env2db(env), stats); if (ret) return ret; } MDB_txn *txn = NULL; @@ -276,7 +289,7 @@ static void cdb_close_env(struct lmdb_env *env, struct kr_cdb_stats *stats) /* Get rid of any transactions. */ txn_free_ro(env); - cdb_commit(env, stats); + cdb_commit(env2db(env), stats); mdb_env_sync(env->env, 1); stats->close++; @@ -388,7 +401,7 @@ error_sys: return kr_error(ret); } -static int cdb_init(knot_db_t **db, struct kr_cdb_stats *stats, +static int cdb_init(kr_cdb_pt *db, struct kr_cdb_stats *stats, struct kr_cdb_opts *opts, knot_mm_t *pool) { if (!db || !stats || !opts) { @@ -406,20 +419,19 @@ static int cdb_init(knot_db_t **db, struct kr_cdb_stats *stats, return ret; } - *db = env; + *db = env2db(env); return 0; } -static void cdb_deinit(knot_db_t *db, struct kr_cdb_stats *stats) +static void cdb_deinit(kr_cdb_pt db, struct kr_cdb_stats *stats) { - struct lmdb_env *env = db; - cdb_close_env(env, stats); - free(env); + cdb_close_env(db2env(db), stats); + free(db); } -static int cdb_count(knot_db_t *db, struct kr_cdb_stats *stats) +static int cdb_count(kr_cdb_pt db, struct kr_cdb_stats *stats) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); MDB_txn *txn = NULL; int ret = txn_get(env, &txn, true); if (ret != 0) { @@ -451,9 +463,9 @@ static int reopen_env(struct lmdb_env *env, struct kr_cdb_stats *stats, const si return cdb_open_env(env, path_copy, mapsize, stats); } -static int cdb_check_health(knot_db_t *db, struct kr_cdb_stats *stats) +static int cdb_check_health(kr_cdb_pt db, struct kr_cdb_stats *stats) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); struct stat st; if (stat(env->mdb_data_path, &st)) { @@ -518,9 +530,9 @@ static int lockfile_release(const char *path, int fd) return err; } -static int cdb_clear(knot_db_t *db, struct kr_cdb_stats *stats) +static int cdb_clear(kr_cdb_pt db, struct kr_cdb_stats *stats) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); stats->clear++; /* First try mdb_drop() to clear the DB; this may fail with ENOSPC. */ { @@ -542,7 +554,7 @@ static int cdb_clear(knot_db_t *db, struct kr_cdb_stats *stats) * though it's best for them to reopen soon. */ /* We are about to switch to a different file, so end all txns, to be sure. */ - txn_free_ro(db); + txn_free_ro(env); (void) cdb_commit(db, stats); const char *path = NULL; @@ -591,10 +603,10 @@ static int cdb_clear(knot_db_t *db, struct kr_cdb_stats *stats) return ret; } -static int cdb_readv(knot_db_t *db, struct kr_cdb_stats *stats, +static int cdb_readv(kr_cdb_pt db, struct kr_cdb_stats *stats, const knot_db_val_t *key, knot_db_val_t *val, int maxcount) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); MDB_txn *txn = NULL; int ret = txn_get(env, &txn, true); if (ret) { @@ -648,10 +660,10 @@ static int cdb_write(struct lmdb_env *env, MDB_txn **txn, const knot_db_val_t *k return kr_ok(); } -static int cdb_writev(knot_db_t *db, struct kr_cdb_stats *stats, +static int cdb_writev(kr_cdb_pt db, struct kr_cdb_stats *stats, const knot_db_val_t *key, knot_db_val_t *val, int maxcount) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); MDB_txn *txn = NULL; int ret = txn_get(env, &txn, false); @@ -671,10 +683,10 @@ static int cdb_writev(knot_db_t *db, struct kr_cdb_stats *stats, return ret; } -static int cdb_remove(knot_db_t *db, struct kr_cdb_stats *stats, +static int cdb_remove(kr_cdb_pt db, struct kr_cdb_stats *stats, knot_db_val_t keys[], int maxcount) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); MDB_txn *txn = NULL; int ret = txn_get(env, &txn, false); int deleted = 0; @@ -698,10 +710,10 @@ static int cdb_remove(knot_db_t *db, struct kr_cdb_stats *stats, return ret < 0 ? ret : deleted; } -static int cdb_match(knot_db_t *db, struct kr_cdb_stats *stats, +static int cdb_match(kr_cdb_pt db, struct kr_cdb_stats *stats, knot_db_val_t *key, knot_db_val_t keyval[][2], int maxcount) { - struct lmdb_env *env = db; + struct lmdb_env *env = db2env(db); MDB_txn *txn = NULL; int ret = txn_get(env, &txn, true); if (ret != 0) { @@ -757,10 +769,11 @@ static int cdb_match(knot_db_t *db, struct kr_cdb_stats *stats, } -static int cdb_read_leq(knot_db_t *env, struct kr_cdb_stats *stats, +static int cdb_read_leq(kr_cdb_pt db, struct kr_cdb_stats *stats, knot_db_val_t *key, knot_db_val_t *val) { - assert(env && key && key->data && val); + assert(db && key && key->data && val); + struct lmdb_env *env = db2env(db); MDB_cursor *curs = NULL; int ret = txn_curs_get(env, &curs, stats); if (ret) return ret; @@ -806,17 +819,16 @@ static double cdb_usage(knot_db_t *db) return db_usage; } -static size_t cdb_get_maxsize(knot_db_t *db) +static size_t cdb_get_maxsize(kr_cdb_pt db) { - struct lmdb_env *env = db; - return env->mapsize; + return db2env(db)->mapsize; } /** Conversion between knot and lmdb structs. */ -knot_db_t *knot_db_t_kres2libknot(const knot_db_t * db) +knot_db_t *kr_cdb_pt2knot_db_t(kr_cdb_pt db) { /* this is struct lmdb_env as in resolver/cdb_lmdb.c */ - const struct lmdb_env *kres_db = db; + const struct lmdb_env *kres_db = db2env(db); struct libknot_lmdb_env *libknot_db = malloc(sizeof(*libknot_db)); if (libknot_db != NULL) { libknot_db->shared = false; diff --git a/lib/cache/cdb_lmdb.h b/lib/cache/cdb_lmdb.h index 020e05690..3429a222c 100644 --- a/lib/cache/cdb_lmdb.h +++ b/lib/cache/cdb_lmdb.h @@ -10,5 +10,7 @@ KR_EXPORT KR_CONST const struct kr_cdb_api *kr_cdb_lmdb(void); +/** Create a pointer for knot_db_lmdb_api. You free() it to release it. */ KR_EXPORT -knot_db_t *knot_db_t_kres2libknot(const knot_db_t * db); +knot_db_t *kr_cdb_pt2knot_db_t(kr_cdb_pt db); + diff --git a/utils/cache_gc/db.c b/utils/cache_gc/db.c index 573f2ba0b..0ab829b9e 100644 --- a/utils/cache_gc/db.c +++ b/utils/cache_gc/db.c @@ -30,7 +30,7 @@ int kr_gc_cache_open(const char *cache_path, struct kr_cache *kres_db, return -EINVAL; } - *libknot_db = knot_db_t_kres2libknot(kres_db->db); + *libknot_db = kr_cdb_pt2knot_db_t(kres_db->db); if (*libknot_db == NULL) { printf("Out of memory.\n"); return -ENOMEM; @@ -50,7 +50,7 @@ int kr_gc_cache_check_health(struct kr_cache *kres_db, knot_db_t ** libknot_db) } /* Cache was reopen. */ free(*libknot_db); - *libknot_db = knot_db_t_kres2libknot(kres_db->db); + *libknot_db = kr_cdb_pt2knot_db_t(kres_db->db); if (*libknot_db == NULL) { printf("Out of memory.\n"); return -ENOMEM; -- 2.47.2