From: Chris Luke Date: Sat, 30 Jul 2016 19:05:07 +0000 (-0400) Subject: VPP-189 Tweak hash_foreach_pair to avoid static warning X-Git-Tag: v17.01-rc0~267 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;ds=sidebyside;h=5cdaf6358266eb883a2e48239a83e9f56c3c5bad;p=vpp.git VPP-189 Tweak hash_foreach_pair to avoid static warning Coverity doesn't like when an incrementing pointer is taken from the address of a singleton; it cries that this is a potential buffer overrun. Since the cases of this in hash_foreach_pair are based on items from a simple union used just to have different types point to the same location it's a simple matter of using the pointer to that location directly. Since we then aren't using the members of the union, we can change that to an opaque pointer (void *). This accounts for >60 issues in Coverity. Whilst here, convert some useful existing comments into a docblock. Change-Id: I114183ab7d7948d4a6a703451417f79fa37634eb Signed-off-by: Chris Luke --- diff --git a/vppinfra/vppinfra/hash.h b/vppinfra/vppinfra/hash.h index f796f2dca09..4db5a57602e 100644 --- a/vppinfra/vppinfra/hash.h +++ b/vppinfra/vppinfra/hash.h @@ -329,21 +329,32 @@ hash_forward (hash_t * h, void *v, uword n) return (u8 *) v + ((n * sizeof (hash_pair_t)) << h->log2_pair_size); } -/* Iterate over hash pairs - @param p the current (key,value) pair - @param v the hash table to iterate - @param body the operation to perform on each (key,value) pair. - executes body with each active hash pair +/** Iterate over hash pairs. + + @param p The current (key,value) pair. This should be of type + (hash_pair_t *). + @param v The hash table to iterate. + @param body The operation to perform on each (key,value) pair. + + Executes the expression or code block @c body with each active hash pair. */ +/* A previous version of this macro made use of the hash_pair_union_t + * structure; this version does not since that approach mightily upset + * the static analysis tool. In the rare chance someone is reading this + * code, pretend that _p below is of type hash_pair_union_t and that when + * used as an rvalue it's really using one of the union members as the + * rvalue. If you were confused before you might be marginally less + * confused after. + */ #define hash_foreach_pair(p,v,body) \ do { \ __label__ _hash_foreach_done; \ hash_t * _h = hash_header (v); \ - hash_pair_union_t * _p; \ + void * _p; \ hash_pair_t * _q, * _q_end; \ uword _i, _i1, _id, _pair_increment; \ \ - _p = (hash_pair_union_t *) (v); \ + _p = (v); \ _i = 0; \ _pair_increment = 1; \ if ((v)) \ @@ -356,12 +367,12 @@ do { \ do { \ if (_id & 1) \ { \ - _q = &_p->direct; \ + _q = _p; \ _q_end = _q + _pair_increment; \ } \ else \ { \ - hash_pair_indirect_t * _pi = &_p->indirect; \ + hash_pair_indirect_t * _pi = _p; \ _q = _pi->pairs; \ if (_h->log2_pair_size > 0) \ _q_end = hash_forward (_h, _q, indirect_pair_get_len (_pi)); \ @@ -384,7 +395,7 @@ do { \ _q += _pair_increment; \ } \ \ - _p = (hash_pair_union_t *) (&_p->direct + _pair_increment); \ + _p = (hash_pair_t *)_p + _pair_increment; \ _id = _id / 2; \ _i++; \ } while (_i < _i1); \