Switch Coverity-only code to assert (CID #1619299) (#5441)

fr_nbo_from_uint64v() does not have an error return--it doesn't
need one. The buffers are big enough, the "| 0x80" means it will
always use as least one byte, so fr_high_bit_pos() won't return 0
even if num == 0. So adding a bogus error return check for Coverity
actually misleads Coverity about any call to fr_nbo_from_uint64v(),
making it the probable cause of the CID.

Co-authored-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org>

diff --git a/src/lib/util/nbo.h b/src/lib/util/nbo.h
index 25eb27f987762d87037125b39dfa6ee3df20993b..627eef671c7cfdd0e2cd2a68446f63a52d19df78 100644 (file)
--- a/src/lib/util/nbo.h
+++ b/src/lib/util/nbo.h
@@ -123,10 +123,12 @@ static inline size_t fr_nbo_from_uint64v(uint8_t out[static sizeof(uint64_t)], u
        ret = ROUND_UP_DIV((size_t)fr_high_bit_pos(num | 0x80), 8);
 #ifdef __COVERITY__
        /*
-        *      Coverity doesn't realize that ret is necessarily <= 8,
-        *      so we give it a hint.
+        * Coverity doesn't realize that the fr_high_bit_pos() call will always
+        * return a value between 1 and 8 inclusive, the former thanks to the
+        * "| 0x80". and this function doesn't specify an error return value,
+        * so we use a Coverity-only assert.
         */
-       if (ret > 8) return 1;
+       if (!fr_cond_assert((ret >= 1) && (ret <= 8))) return 1;
 #endif
 
        fr_nbo_from_uint64(swapped, num);