Browse Source

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.
Rusty Russell 15 years ago
parent
commit
09fde153ba
4 changed files with 34 additions and 31 deletions
  1. 2 2
      ccan/jmap/jmap.c
  2. 22 22
      ccan/jmap/jmap.h
  3. 5 2
      ccan/jmap/jmap_type.h
  4. 5 5
      ccan/jmap/test/run-ptridx-type.c

+ 2 - 2
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) {

+ 22 - 22
ccan/jmap/jmap.h

@@ -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;
 }

+ 5 - 2
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)		\

+ 5 - 5
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);