jmap: fix aliasing issues, one real testcase bug
authorRusty Russell <rusty@rustcorp.com.au>
Wed, 3 Nov 2010 00:11:07 +0000 (10:41 +1030)
committerRusty Russell <rusty@rustcorp.com.au>
Wed, 3 Nov 2010 00:11:07 +0000 (10:41 +1030)
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
ccan/jmap/jmap.h
ccan/jmap/jmap_type.h
ccan/jmap/test/run-ptridx-type.c

index 279f65192af47c6e57f2698bb2951884363d3e62..afac2fc9ff3adb1bc83f44ee26566be5c439f182 100644 (file)
@@ -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) {
index faba53c024beee6927d0d8256d2b3f1b69505471..93c03b9b03bbf3b0f043bf8d5cc456160f5adbe7 100644 (file)
@@ -1,7 +1,11 @@
 #ifndef CCAN_JMAP_H
 #define CCAN_JMAP_H
+#include <stddef.h>
+#define _WORD_T
+typedef size_t Word_t, *PWord_t;
 #include <Judy.h>
 #include <stdbool.h>
+#include <string.h>
 #include <ccan/compiler/compiler.h>
 #include <assert.h>
 #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;
 }
index dbbb97bf28d3cc0109ebd8eb17ff37b4431dc43c..e61ba10946d260083c96207217b4d61eda2f2179 100644 (file)
@@ -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)               \
index 83642a8e68c36d7f7623199795e8eea0fe2d6d6c..aa715cf51162b70f80ed3a18d14795a69b4ed9ae 100644 (file)
@@ -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);