[ccan] clang warnings from ccan/rbtree/rbtree.c

Brad Hards bradh at frogmouth.net
Tue Mar 8 16:29:54 EST 2011


I've been running the clang static analysis tool over ccan. Some issues, some 
false alarms, some "hmm, what now".

Here is an extract from ccan/rbtree/rbtree.c:

/* setting a NULL node to black is a nop */
static inline void trbt_set_color(trbt_node_t *node, int color)
{
	if ( (node==NULL) && (color==TRBT_BLACK) ) {
		return;
	}
	node->rb_color = color;
}
static inline void trbt_set_color_left(trbt_node_t *node, int color)
{
	if ( ((node==NULL)||(node->left==NULL)) && (color==TRBT_BLACK) ) {
		return;
	}
	node->left->rb_color = color;
}
static inline void trbt_set_color_right(trbt_node_t *node, int color)
{
	if ( ((node==NULL)||(node->right==NULL)) && (color==TRBT_BLACK) ) {
		return;
	}
	node->right->rb_color = color;
}

It only complains about the last one:
216  static inline void trbt_set_color_right(trbt_node_t *node, int color)
217  {
218          if ( ((node==NULL)||(node->right==NULL)) && (color==TRBT_BLACK) ) 
{
  1.  Assuming pointer value is null
  2. Taking false branch
219                  return;
220          }
221          node->right->rb_color = color;
  3.  Dereference of null pointer
222  }

That is, if node or node->right are NULL, and color is not equal to 
TRBT_BLACK, then it will segfault. 

Now, I couldn't find a case where we ever pass anything other than a color of 
TRBT_BLACK, but I'm still not sure what to do here:
1. Simplify the function so it only ever sets to black?
2. Change it to || instead of &&?
3. Ignore the whole thing?

Thoughts? Suggestions?

Brad


More information about the ccan mailing list