From 09fde153ba7a68715dcad3b53cbbb8804c3d2356 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 3 Nov 2010 10:41:07 +1030 Subject: [PATCH] jmap: fix aliasing issues, one real testcase bug Compiling with gcc -O3 added some more warnings, especially about aliasing. By setting Word_t to size_t we fix some of them, but jmap_@name_firstval in JMAP_DEFINE_PTRIDX_TYPE we need to use a real size_t rather than lazily casting our index to a size_t *. Gcc also spotted taht we used idx[NUM] in test/run-ptridx-type.c; allocate that and make the usage explicit. --- ccan/jmap/jmap.c | 4 +-- ccan/jmap/jmap.h | 44 ++++++++++++++++---------------- ccan/jmap/jmap_type.h | 7 +++-- ccan/jmap/test/run-ptridx-type.c | 10 ++++---- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/ccan/jmap/jmap.c b/ccan/jmap/jmap.c index 279f6519..afac2fc9 100644 --- a/ccan/jmap/jmap.c +++ b/ccan/jmap/jmap.c @@ -7,8 +7,8 @@ struct jmap *jmap_new(void) { struct jmap *map; - /* Judy uses Word_t, we use size_t. */ - BUILD_ASSERT(sizeof(size_t) == sizeof(Word_t)); + /* Judy uses unsigned long for Word_t, we use size_t. */ + BUILD_ASSERT(sizeof(size_t) == sizeof(unsigned long)); map = malloc(sizeof(*map)); if (map) { diff --git a/ccan/jmap/jmap.h b/ccan/jmap/jmap.h index faba53c0..93c03b9b 100644 --- a/ccan/jmap/jmap.h +++ b/ccan/jmap/jmap.h @@ -1,7 +1,11 @@ #ifndef CCAN_JMAP_H #define CCAN_JMAP_H +#include +#define _WORD_T +typedef size_t Word_t, *PWord_t; #include #include +#include #include #include #ifdef DEBUG @@ -68,7 +72,7 @@ static inline void jmap_debug_del_access(struct jmap *map, size_t **val) map->acc_value = NULL; #endif /* Set it to some invalid value. Not NULL, they might rely on that! */ - assert((*val = (void *)jmap_new) != NULL); + assert(memset(val, 0x42, sizeof(*val))); } static inline void jmap_debug_access(struct jmap *map) @@ -126,9 +130,9 @@ static inline const char *jmap_error(struct jmap *map) */ static inline bool jmap_add(struct jmap *map, size_t index, size_t value) { - Word_t *val; + size_t *val; jmap_debug_access(map); - val = (void *)JudyLIns(&map->judy, index, &map->err); + val = (size_t *)JudyLIns(&map->judy, index, &map->err); if (val == PJERR) return false; *val = value; @@ -150,8 +154,8 @@ static inline bool jmap_add(struct jmap *map, size_t index, size_t value) */ static inline bool jmap_set(const struct jmap *map, size_t index, size_t value) { - Word_t *val; - val = (void *)JudyLGet(map->judy, index, (JError_t *)&map->err); + size_t *val; + val = (size_t *)JudyLGet(map->judy, index, (JError_t *)&map->err); if (val && val != PJERR) { *val = value; return true; @@ -204,8 +208,8 @@ static inline bool jmap_test(const struct jmap *map, size_t index) static inline size_t jmap_get(const struct jmap *map, size_t index, size_t invalid) { - Word_t *val; - val = (void *)JudyLGet(map->judy, index, (JError_t *)&map->err); + size_t *val; + val = (size_t *)JudyLGet(map->judy, index, (JError_t *)&map->err); if (!val || val == PJERR) return invalid; return *val; @@ -252,7 +256,7 @@ static inline size_t jmap_popcount(const struct jmap *map, static inline size_t jmap_nth(const struct jmap *map, size_t n, size_t invalid) { - Word_t index; + size_t index; if (!JudyLByCount(map->judy, n+1, &index, (JError_t *)&map->err)) index = invalid; return index; @@ -277,7 +281,7 @@ static inline size_t jmap_nth(const struct jmap *map, */ static inline size_t jmap_first(const struct jmap *map, size_t invalid) { - Word_t index = 0; + size_t index = 0; if (!JudyLFirst(map->judy, &index, (JError_t *)&map->err)) index = invalid; else @@ -298,7 +302,7 @@ static inline size_t jmap_first(const struct jmap *map, size_t invalid) static inline size_t jmap_next(const struct jmap *map, size_t prev, size_t invalid) { - if (!JudyLNext(map->judy, (Word_t *)&prev, (JError_t *)&map->err)) + if (!JudyLNext(map->judy, &prev, (JError_t *)&map->err)) prev = invalid; else assert(prev != invalid); @@ -321,7 +325,7 @@ static inline size_t jmap_next(const struct jmap *map, size_t prev, */ static inline size_t jmap_last(const struct jmap *map, size_t invalid) { - Word_t index = -1; + size_t index = -1; if (!JudyLLast(map->judy, &index, (JError_t *)&map->err)) index = invalid; else @@ -342,7 +346,7 @@ static inline size_t jmap_last(const struct jmap *map, size_t invalid) static inline size_t jmap_prev(const struct jmap *map, size_t prev, size_t invalid) { - if (!JudyLPrev(map->judy, (Word_t *)&prev, (JError_t *)&map->err)) + if (!JudyLPrev(map->judy, &prev, (JError_t *)&map->err)) prev = invalid; else assert(prev != invalid); @@ -380,7 +384,7 @@ static inline size_t jmap_prev(const struct jmap *map, size_t prev, static inline size_t *jmap_getval(struct jmap *map, size_t index) { size_t *val; - val = (void *)JudyLGet(map->judy, index, (JError_t *)&map->err); + val = (size_t *)JudyLGet(map->judy, index, (JError_t *)&map->err); jmap_debug_add_access(map, index, val, "jmap_getval"); return val; } @@ -432,7 +436,7 @@ static inline size_t *jmap_nthval(const struct jmap *map, size_t n, size_t *index) { size_t *val; - val = (size_t *)JudyLByCount(map->judy, n+1, (Word_t *)index, + val = (size_t *)JudyLByCount(map->judy, n+1, index, (JError_t *)&map->err); jmap_debug_add_access(map, *index, val, "jmap_nthval"); return val; @@ -461,8 +465,7 @@ static inline size_t *jmap_firstval(const struct jmap *map, size_t *index) { size_t *val; *index = 0; - val = (size_t *)JudyLFirst(map->judy, (Word_t *)index, - (JError_t *)&map->err); + val = (size_t *)JudyLFirst(map->judy, index, (JError_t *)&map->err); jmap_debug_add_access(map, *index, val, "jmap_firstval"); return val; } @@ -481,8 +484,7 @@ static inline size_t *jmap_firstval(const struct jmap *map, size_t *index) static inline size_t *jmap_nextval(const struct jmap *map, size_t *index) { size_t *val; - val = (size_t *)JudyLNext(map->judy, (Word_t *)index, - (JError_t *)&map->err); + val = (size_t *)JudyLNext(map->judy, index, (JError_t *)&map->err); jmap_debug_add_access(map, *index, val, "jmap_nextval"); return val; } @@ -499,8 +501,7 @@ static inline size_t *jmap_lastval(const struct jmap *map, size_t *index) { size_t *val; *index = -1; - val = (size_t *)JudyLLast(map->judy, (Word_t *)index, - (JError_t *)&map->err); + val = (size_t *)JudyLLast(map->judy, index, (JError_t *)&map->err); jmap_debug_add_access(map, *index, val, "jmap_lastval"); return val; } @@ -519,8 +520,7 @@ static inline size_t *jmap_lastval(const struct jmap *map, size_t *index) static inline size_t *jmap_prevval(const struct jmap *map, size_t *index) { size_t *val; - val = (size_t *)JudyLPrev(map->judy, (Word_t *)index, - (JError_t *)&map->err); + val = (size_t *)JudyLPrev(map->judy, index, (JError_t *)&map->err); jmap_debug_add_access(map, *index, val, "jmap_prevval"); return val; } diff --git a/ccan/jmap/jmap_type.h b/ccan/jmap/jmap_type.h index dbbb97bf..e61ba109 100644 --- a/ccan/jmap/jmap_type.h +++ b/ccan/jmap/jmap_type.h @@ -264,8 +264,11 @@ static inline void jmap_##name##_putval(struct jmap_##name *map, \ static inline type **jmap_##name##_firstval(const struct jmap_##name *map, \ itype **index) \ { \ - return (type **)jmap_firstval((const struct jmap *)map, \ - (size_t *)index); \ + size_t idx; \ + type **ret; \ + ret = (type **)jmap_firstval((const struct jmap *)map, &idx); \ + *index = (void *)idx; \ + return ret; \ } \ static inline type **jmap_##name##_nextval(const struct jmap_##name *map, \ itype **index) \ diff --git a/ccan/jmap/test/run-ptridx-type.c b/ccan/jmap/test/run-ptridx-type.c index 83642a8e..aa715cf5 100644 --- a/ccan/jmap/test/run-ptridx-type.c +++ b/ccan/jmap/test/run-ptridx-type.c @@ -18,25 +18,25 @@ int main(int argc, char *argv[]) { struct jmap_foo *map; struct foo *foo[NUM], **foop; - struct idx *idx[NUM], *index; + struct idx *idx[NUM+1], *index; unsigned int i; plan_tests(25 + NUM*2 + 6); - for (i = 0; i < NUM; i++) + for (i = 0; i < NUM+1; i++) foo[i] = malloc(20); qsort(foo, NUM, sizeof(foo[0]), cmp_ptr); /* idx[i] == foo[i] + 1, for easy checking */ - for (i = 0; i < NUM; i++) + for (i = 0; i < NUM+1; i++) idx[i] = (void *)((char *)foo[i] + 1); map = jmap_foo_new(); ok1(jmap_foo_error(map) == NULL); - ok1(jmap_foo_test(map, idx[i]) == false); - ok1(jmap_foo_get(map, idx[i]) == (struct foo *)NULL); + ok1(jmap_foo_test(map, idx[NUM]) == false); + ok1(jmap_foo_get(map, idx[NUM]) == (struct foo *)NULL); ok1(jmap_foo_count(map) == 0); ok1(jmap_foo_first(map) == (struct idx *)NULL); ok1(jmap_foo_del(map, idx[0]) == false); -- 2.39.2