Skip to content

Commit

Permalink
cache: support string without a NUL terminator
Browse files Browse the repository at this point in the history
Also we, by which I mean me, might have been using HASH_ADD_STR wrong...

Signed-off-by: Yuxuan Shui <[email protected]>
  • Loading branch information
yshui committed Feb 18, 2024
1 parent 664f8b8 commit 5fa870a
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 24 deletions.
16 changes: 8 additions & 8 deletions src/atom.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}

/**
Expand All @@ -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
Expand Down
11 changes: 9 additions & 2 deletions src/atom.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
2 changes: 1 addition & 1 deletion src/c2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 10 additions & 8 deletions src/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
8 changes: 4 additions & 4 deletions src/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/picom.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 5fa870a

Please sign in to comment.