VPP-189 Tweak hash_foreach_pair to avoid static warning 44/2144/5
authorChris Luke <chrisy@flirble.org>
Sat, 30 Jul 2016 19:05:07 +0000 (15:05 -0400)
committerDave Barach <openvpp@barachs.net>
Wed, 31 Aug 2016 12:57:33 +0000 (12:57 +0000)
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 <chrisy@flirble.org>
vppinfra/vppinfra/hash.h

index f796f2d..4db5a57 100644 (file)
@@ -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
+                <code>(hash_pair_t *)</code>.
+    @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);                                                   \