lib/cache: type safety of the cache API pointers

See the definition of kr_cdb_pt for details.

diff --git a/daemon/bindings/cache.c b/daemon/bindings/cache.c
index 49cb978a207f92966eb3fffb918f71e4726d6a2e..56b75fc937b233728100758728ba910693257346 100644 (file)
--- 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 7ba576f15002960011b7f82ed38a0d3fc5159c4c..eef26bfd7b3bd8b7c9fefca4d40be9a669703b48 100644 (file)
--- 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 13070335230d120048bfabebe0ca110bdcea2769..bd5c1fd4618f1caa250a88581c8492a0f129ce26 100755 (executable)
--- 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 e0786638d092378adb8b6e7304614572f282ce65..28b3eb6effa854a0a48bf376c644e929cb9b1a38 100644 (file)
--- 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 546964fde9a871bc0d5fccd7fd8f687e372e30d0..33ec2ffa296a8bbb231b3c578b9866141d9bb053 100644 (file)
--- 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 e458e496865c277d2c9f5fa509145d7b59468d8d..ff152fb7ac9f1178d87bb4983bdafdcbaf148016 100644 (file)
--- 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 020e056901c026d252cae42f65ecc2ed3cc5e4d3..3429a222c7288ab881bb9e1d006e9c60a494f30f 100644 (file)
--- 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 573f2ba0b07bd6219ad56496d462931a9bdb3f77..0ab829b9e95459b48246afc08316b0df46e02909 100644 (file)
--- 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;