diff --git a/src/atom.c b/src/atom.c index bf94232e..76fc57f4 100644 --- a/src/atom.c +++ b/src/atom.c @@ -2,14 +2,19 @@ #include #include "atom.h" +#include "cache.h" #include "common.h" -#include "utils.h" +#include "compiler.h" #include "log.h" +#include "utils.h" -static inline void *atom_getter(void *ud, const char *atom_name, int *err) { - xcb_connection_t *c = ud; +static inline int +atom_getter(struct cache *cache, const char *atom_name, struct cache_handle **value) { + struct atom *atoms = container_of(cache, struct atom, c); xcb_intern_atom_reply_t *reply = xcb_intern_atom_reply( - c, xcb_intern_atom(c, 0, to_u16_checked(strlen(atom_name)), atom_name), NULL); + atoms->conn, + xcb_intern_atom(atoms->conn, 0, to_u16_checked(strlen(atom_name)), atom_name), + NULL); xcb_atom_t atom = XCB_NONE; if (reply) { @@ -18,9 +23,19 @@ static inline void *atom_getter(void *ud, const char *atom_name, int *err) { free(reply); } else { log_error("Failed to intern atoms"); - *err = 1; + return -1; } - return (void *)(intptr_t)atom; + + struct atom_entry *entry = ccalloc(1, struct atom_entry); + entry->atom = atom; + *value = &entry->entry; + return 0; +} + +static inline void +atom_entry_free(struct cache *cache attr_unused, struct cache_handle *handle) { + struct atom_entry *entry = cache_entry(handle, struct atom_entry, entry); + free(entry); } /** @@ -28,11 +43,16 @@ static inline void *atom_getter(void *ud, const char *atom_name, int *err) { */ struct atom *init_atoms(xcb_connection_t *c) { auto atoms = ccalloc(1, struct atom); - atoms->c = new_cache((void *)c, atom_getter, NULL); -#define ATOM_GET(x) \ - atoms->a##x = (xcb_atom_t)(intptr_t)cache_get_or_fetch(atoms->c, #x, NULL) + atoms->conn = c; + cache_init(&atoms->c, atom_getter); +#define ATOM_GET(x) atoms->a##x = get_atom(atoms, #x) LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST2); #undef ATOM_GET return atoms; } + +void destroy_atoms(struct atom *a) { + cache_invalidate_all(&a->c, atom_entry_free); + free(a); +} diff --git a/src/atom.h b/src/atom.h index 91c28290..67873317 100644 --- a/src/atom.h +++ b/src/atom.h @@ -4,6 +4,7 @@ #include #include "cache.h" +#include "log.h" #include "meta.h" // clang-format off @@ -54,22 +55,33 @@ #define ATOM_DEF(x) xcb_atom_t a##x struct atom { - struct cache *c; + xcb_connection_t *conn; + struct cache c; LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_DEF, SEP_COLON, ATOM_LIST2); }; +struct atom_entry { + struct cache_handle entry; + xcb_atom_t atom; +}; + +/// Create a new atom object with a xcb connection, note that this atom object will hold a +/// reference to the connection, so the caller must keep the connection alive until the +/// atom object is destroyed. struct atom *init_atoms(xcb_connection_t *); static inline xcb_atom_t get_atom(struct atom *a, const char *key) { - return (xcb_atom_t)(intptr_t)cache_get_or_fetch(a->c, key, NULL); + struct cache_handle *entry = NULL; + if (cache_get_or_fetch(&a->c, key, &entry) < 0) { + log_error("Failed to get atom %s", key); + return XCB_NONE; + } + return cache_entry(entry, struct atom_entry, entry)->atom; } static inline xcb_atom_t get_atom_cached(struct atom *a, const char *key) { - return (xcb_atom_t)(intptr_t)cache_get(a->c, key); + return cache_entry(cache_get(&a->c, key), struct atom_entry, entry)->atom; } -static inline void destroy_atoms(struct atom *a) { - cache_free(a->c); - free(a); -} +void destroy_atoms(struct atom *a); diff --git a/src/cache.c b/src/cache.c index 8c5d91d6..d25a22fb 100644 --- a/src/cache.c +++ b/src/cache.c @@ -1,94 +1,56 @@ #include -#include "compiler.h" -#include "utils.h" #include "cache.h" -struct cache_entry { - char *key; - void *value; - UT_hash_handle hh; -}; - -struct cache { - cache_getter_t getter; - cache_free_t free; - void *user_data; - struct cache_entry *entries; -}; - -static inline struct cache_entry *cache_get_entry(struct cache *c, const char *key) { - struct cache_entry *e; +struct cache_handle *cache_get(struct cache *c, const char *key) { + struct cache_handle *e; HASH_FIND_STR(c->entries, key, e); return e; } -void *cache_get(struct cache *c, const char *key) { - struct cache_entry *e = cache_get_entry(c, key); - return e ? e->value : NULL; +int cache_get_or_fetch(struct cache *c, const char *key, struct cache_handle **value) { + *value = cache_get(c, key); + if (*value) { + return 0; + } + + int err = c->getter(c, key, value); + assert(err <= 0); + if (err < 0) { + return err; + } + (*value)->key = strdup(key); + + HASH_ADD_STR(c->entries, key, *value); + return 1; } -void *cache_get_or_fetch(struct cache *c, const char *key, int *err) { - struct cache_entry *e = cache_get_entry(c, key); - if (e) { - return e->value; - } - - int tmperr; - if (!err) { - err = &tmperr; - } - - *err = 0; - e = ccalloc(1, struct cache_entry); - e->key = strdup(key); - e->value = c->getter(c->user_data, key, err); - if (*err) { - free(e->key); - free(e); - return NULL; - } - - HASH_ADD_STR(c->entries, key, e); - return e->value; -} - -static inline void cache_invalidate_impl(struct cache *c, struct cache_entry *e) { - if (c->free) { - c->free(c->user_data, e->value); - } +static inline void +cache_invalidate_impl(struct cache *c, struct cache_handle *e, cache_free_t free_fn) { free(e->key); HASH_DEL(c->entries, e); - free(e); + if (free_fn) { + free_fn(c, e); + } } -void cache_invalidate(struct cache *c, const char *key) { - struct cache_entry *e; +void cache_invalidate(struct cache *c, const char *key, cache_free_t free_fn) { + struct cache_handle *e; HASH_FIND_STR(c->entries, key, e); if (e) { - cache_invalidate_impl(c, e); + cache_invalidate_impl(c, e, free_fn); } } -void cache_invalidate_all(struct cache *c) { - struct cache_entry *e, *tmpe; +void cache_invalidate_all(struct cache *c, cache_free_t free_fn) { + struct cache_handle *e, *tmpe; HASH_ITER(hh, c->entries, e, tmpe) { - cache_invalidate_impl(c, e); + cache_invalidate_impl(c, e, free_fn); } } -void *cache_free(struct cache *c) { - void *ret = c->user_data; - cache_invalidate_all(c); - free(c); - return ret; -} - -struct cache *new_cache(void *ud, cache_getter_t getter, cache_free_t f) { - auto c = ccalloc(1, struct cache); - c->user_data = ud; - c->getter = getter; - c->free = f; - return c; +void cache_init(struct cache *cache, cache_getter_t getter) { + cache->getter = getter; + cache->entries = NULL; } diff --git a/src/cache.h b/src/cache.h index 2ac1a01b..60609190 100644 --- a/src/cache.h +++ b/src/cache.h @@ -1,30 +1,46 @@ #pragma once +#include +#include "utils.h" + +#define cache_entry(ptr, type, member) container_of(ptr, type, member) + struct cache; +struct cache_handle; -typedef void *(*cache_getter_t)(void *user_data, const char *key, int *err); -typedef void (*cache_free_t)(void *user_data, void *data); +/// User-provided function to fetch a value for the cache, when it's not present. +/// Should return 0 if the value is fetched successfully, and a negative number if the +/// value cannot be fetched. Getter doesn't need to initialize fields of `struct +/// cache_handle`. +typedef int (*cache_getter_t)(struct cache *, const char *key, struct cache_handle **value); +typedef void (*cache_free_t)(struct cache *, struct cache_handle *value); -/// Create a cache with `getter`, and a free function `f` which is used to free the cache -/// value when they are invalidated. -/// -/// `user_data` will be passed to `getter` and `f` when they are called. -struct cache *new_cache(void *user_data, cache_getter_t getter, cache_free_t f); +struct cache { + cache_getter_t getter; + struct cache_handle *entries; +}; + +struct cache_handle { + char *key; + UT_hash_handle hh; +}; + +/// Initialize a cache with `getter` +void cache_init(struct cache *cache, cache_getter_t getter); /// Get a value from the cache. If the value doesn't present in the cache yet, the /// getter will be called, and the returned value will be stored into the cache. -void *cache_get_or_fetch(struct cache *, const char *key, int *err); +/// Returns 0 if the value is already present in the cache, 1 if the value is fetched +/// successfully, and a negative number if the value cannot be fetched. +int cache_get_or_fetch(struct cache *, const char *key, struct cache_handle **value); /// Get a value from the cache. If the value doesn't present in the cache, NULL will be /// returned. -void *cache_get(struct cache *, const char *key); +struct cache_handle *cache_get(struct cache *, const char *key); /// Invalidate a value in the cache. -void cache_invalidate(struct cache *, const char *key); +void cache_invalidate(struct cache *, const char *key, cache_free_t free_fn); -/// Invalidate all values in the cache. -void cache_invalidate_all(struct cache *); - -/// Invalidate all values in the cache and free it. Returns the user data passed to -/// `new_cache` -void *cache_free(struct cache *); +/// Invalidate all values in the cache. After this call, `struct cache` holds no allocated +/// memory, and can be discarded. +void cache_invalidate_all(struct cache *, cache_free_t free_fn); diff --git a/src/list.h b/src/list.h index 19e2c2c2..d63a7646 100644 --- a/src/list.h +++ b/src/list.h @@ -2,18 +2,7 @@ #include #include -/** - * container_of - cast a member of a structure out to the containing structure - * @ptr: the pointer to the member. - * @type: the type of the container struct this is embedded in. - * @member: the name of the member within the struct. - * - */ -#define container_of(ptr, type, member) \ - ({ \ - const __typeof__(((type *)0)->member) *__mptr = (ptr); \ - (type *)((char *)__mptr - offsetof(type, member)); \ - }) +#include "utils.h" struct list_node { struct list_node *next, *prev; diff --git a/src/utils.h b/src/utils.h index 73cb7112..5a42d99a 100644 --- a/src/utils.h +++ b/src/utils.h @@ -117,6 +117,20 @@ safe_isnan(double a) { ASSERT_IN_RANGE(__to_tmp, 0, max); \ (uint32_t) __to_tmp; \ }) + +/** + * container_of - cast a member of a structure out to the containing structure + * @ptr: the pointer to the member. + * @type: the type of the container struct this is embedded in. + * @member: the name of the member within the struct. + * + */ +#define container_of(ptr, type, member) \ + ({ \ + const __typeof__(((type *)0)->member) *__mptr = (ptr); \ + (type *)((char *)__mptr - offsetof(type, member)); \ + }) + /** * Normalize an int value to a specific range. *