From 5fa870ae66be402e313569bf436bbc14b6873065 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Sun, 18 Feb 2024 00:40:07 +0000 Subject: [PATCH] cache: support string without a NUL terminator Also we, by which I mean me, might have been using HASH_ADD_STR wrong... Signed-off-by: Yuxuan Shui --- src/atom.c | 16 ++++++++-------- src/atom.h | 11 +++++++++-- src/c2.c | 2 +- src/cache.c | 18 ++++++++++-------- src/cache.h | 8 ++++---- src/picom.c | 2 +- 6 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/atom.c b/src/atom.c index fdcf73a9..202a2195 100644 --- a/src/atom.c +++ b/src/atom.c @@ -14,14 +14,14 @@ struct atom_entry { }; static inline int atom_getter(struct cache *cache attr_unused, const char *atom_name, - struct cache_handle **value, void *user_data) { + size_t keylen, struct cache_handle **value, void *user_data) { xcb_connection_t *c = user_data; 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); + c, xcb_intern_atom(c, 0, to_u16_checked(keylen), atom_name), NULL); xcb_atom_t atom = XCB_NONE; if (reply) { - log_debug("Atom %s is %d", atom_name, reply->atom); + log_debug("Atom %.*s is %d", (int)keylen, atom_name, reply->atom); atom = reply->atom; free(reply); } else { @@ -41,17 +41,17 @@ atom_entry_free(struct cache *cache attr_unused, struct cache_handle *handle) { free(entry); } -xcb_atom_t get_atom(struct atom *a, const char *key, xcb_connection_t *c) { +xcb_atom_t get_atom(struct atom *a, const char *key, size_t keylen, xcb_connection_t *c) { struct cache_handle *entry = NULL; - if (cache_get_or_fetch(&a->c, key, &entry, c, atom_getter) < 0) { + if (cache_get_or_fetch(&a->c, key, keylen, &entry, c, atom_getter) < 0) { log_error("Failed to get atom %s", key); return XCB_NONE; } return cache_entry(entry, struct atom_entry, entry)->atom; } -xcb_atom_t get_atom_cached(struct atom *a, const char *key) { - return cache_entry(cache_get(&a->c, key), struct atom_entry, entry)->atom; +xcb_atom_t get_atom_cached(struct atom *a, const char *key, size_t keylen) { + return cache_entry(cache_get(&a->c, key, keylen), struct atom_entry, entry)->atom; } /** @@ -60,7 +60,7 @@ xcb_atom_t get_atom_cached(struct atom *a, const char *key) { struct atom *init_atoms(xcb_connection_t *c) { auto atoms = ccalloc(1, struct atom); atoms->c = CACHE_INIT; -#define ATOM_GET(x) atoms->a##x = get_atom(atoms, #x, c) +#define ATOM_GET(x) atoms->a##x = get_atom(atoms, #x, sizeof(#x) - 1, c) LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST1); LIST_APPLY(ATOM_GET, SEP_COLON, ATOM_LIST2); #undef ATOM_GET diff --git a/src/atom.h b/src/atom.h index 0cebe3a0..a0473914 100644 --- a/src/atom.h +++ b/src/atom.h @@ -62,7 +62,14 @@ struct atom { /// a reference to the connection. struct atom *init_atoms(xcb_connection_t *c); -xcb_atom_t get_atom(struct atom *a, const char *key, xcb_connection_t *c); -xcb_atom_t get_atom_cached(struct atom *a, const char *key); +xcb_atom_t get_atom(struct atom *a, const char *key, size_t keylen, xcb_connection_t *c); +static inline xcb_atom_t +get_atom_with_nul(struct atom *a, const char *key, xcb_connection_t *c) { + return get_atom(a, key, strlen(key), c); +} +xcb_atom_t get_atom_cached(struct atom *a, const char *key, size_t keylen); +static inline xcb_atom_t get_atom_cached_with_nul(struct atom *a, const char *key) { + return get_atom_cached(a, key, strlen(key)); +} void destroy_atoms(struct atom *a); diff --git a/src/c2.c b/src/c2.c index 0ca74148..37cd61f3 100644 --- a/src/c2.c +++ b/src/c2.c @@ -1151,7 +1151,7 @@ static bool c2_l_postprocess(struct c2_state *state, xcb_connection_t *c, c2_l_t // Get target atom if it's not a predefined one if (pleaf->predef == C2_L_PUNDEFINED) { - pleaf->tgtatom = get_atom(state->atoms, pleaf->tgt, c); + pleaf->tgtatom = get_atom_with_nul(state->atoms, pleaf->tgt, c); if (!pleaf->tgtatom) { log_error("Failed to get atom for target \"%s\".", pleaf->tgt); return false; diff --git a/src/cache.c b/src/cache.c index e4f2927f..bc67f5b6 100644 --- a/src/cache.c +++ b/src/cache.c @@ -2,27 +2,29 @@ #include "cache.h" -struct cache_handle *cache_get(struct cache *c, const char *key) { +struct cache_handle *cache_get(struct cache *c, const char *key, size_t keylen) { struct cache_handle *e; - HASH_FIND_STR(c->entries, key, e); + HASH_FIND(hh, c->entries, key, keylen, e); return e; } -int cache_get_or_fetch(struct cache *c, const char *key, struct cache_handle **value, - void *user_data, cache_getter_t getter) { - *value = cache_get(c, key); +int cache_get_or_fetch(struct cache *c, const char *key, size_t keylen, + struct cache_handle **value, void *user_data, cache_getter_t getter) { + *value = cache_get(c, key, keylen); if (*value) { return 0; } - int err = getter(c, key, value, user_data); + int err = getter(c, key, keylen, value, user_data); assert(err <= 0); if (err < 0) { return err; } - (*value)->key = strdup(key); + // Add a NUL terminator to make things easier + (*value)->key = cvalloc(keylen + 1); + memcpy((*value)->key, key, keylen); - HASH_ADD_STR(c->entries, key, *value); + HASH_ADD_KEYPTR(hh, c->entries, (*value)->key, keylen, *value); return 1; } diff --git a/src/cache.h b/src/cache.h index 8c022cf4..c0fa8df2 100644 --- a/src/cache.h +++ b/src/cache.h @@ -12,7 +12,7 @@ struct cache_handle; /// 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, +typedef int (*cache_getter_t)(struct cache *, const char *key, size_t keylen, struct cache_handle **value, void *user_data); typedef void (*cache_free_t)(struct cache *, struct cache_handle *value); @@ -31,12 +31,12 @@ struct cache_handle { /// getter will be called, and the returned value will be stored into the cache. /// 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, - void *user_data, cache_getter_t getter); +int cache_get_or_fetch(struct cache *, const char *key, size_t keylen, + struct cache_handle **value, void *user_data, cache_getter_t getter); /// Get a value from the cache. If the value doesn't present in the cache, NULL will be /// returned. -struct cache_handle *cache_get(struct cache *, const char *key); +struct cache_handle *cache_get(struct cache *, const char *key, size_t keylen); /// Invalidate all values in the cache. After this call, `struct cache` holds no allocated /// memory, and can be discarded. diff --git a/src/picom.c b/src/picom.c index 7f5e8df4..0ff437ef 100644 --- a/src/picom.c +++ b/src/picom.c @@ -1345,7 +1345,7 @@ static int register_cm(session_t *ps) { log_fatal("Failed to allocate memory"); return -1; } - atom = get_atom(ps->atoms, buf, ps->c.c); + atom = get_atom_with_nul(ps->atoms, buf, ps->c.c); free(buf); xcb_get_selection_owner_reply_t *reply = xcb_get_selection_owner_reply(