On 12/24/2014 03:32 PM, Bill Fischofer wrote:
The ODP_BITSIZE macro returns the number of bits needed to store x.
For example, ODP_BITSIZE(14) = 4. So if someone specified
ODP_CONFIG_BUFFER_POOLS at 14, then the check would ensure that we
didn't receive a handle with a pool_id of 14 or 15 (which would be
invalid). Because in this case ODP_CONFIG_BUFFER_POOLS is 16 then
Coverity says (correctly) that the test can never be true since a 4
bit value can never be >= 16, however this is a false positive because
the test isn't there for any specific value but for any value of
ODP_CONFIG_BUFFER_POOLS. ODP doesn't require that this be specified
as a power of two. Powers of two are fine for small numbers but
problematic for larger ones, so no need to force this.
The compiler is smart enough to know when the test can never be true
and won't generate any additional code if ODP_CONFIG_BUFFER_POOLS is a
power of two. But if it isn't a power of two then the code will be
generated. So it's there when we need it and not there otherwise,
which is exactly what we want. Coverity just needs to ignore this
because it's doing simple static analysis and not seeing the bigger
picture.
Ok, thanks.
Mike can you mark that as false positive?
Maxim.
On Wed, Dec 24, 2014 at 5:39 AM, Maxim Uvarov <[email protected]
<mailto:[email protected]>> wrote:
On 12/23/2014 04:01 PM, Bill Fischofer wrote:
On Tue, Dec 23, 2014 at 5:39 AM, Maxim Uvarov
<[email protected] <mailto:[email protected]>
<mailto:[email protected]
<mailto:[email protected]>>> wrote:
CID 85006: Operands don't affect result
(CONSTANT_EXPRESSION_RESULT)
"handle.pool_id >= 16" is always false regardless of
the values of
its operands. This occurs as the logical second
operand of '||'.
Signed-off-by: Maxim Uvarov <[email protected]
<mailto:[email protected]>
<mailto:[email protected]
<mailto:[email protected]>>>
---
platform/linux-generic/include/odp_buffer_inlines.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/platform/linux-generic/include/odp_buffer_inlines.h
b/platform/linux-generic/include/odp_buffer_inlines.h
index f880445..a28e1f1 100644
--- a/platform/linux-generic/include/odp_buffer_inlines.h
+++ b/platform/linux-generic/include/odp_buffer_inlines.h
@@ -99,8 +99,8 @@ static inline odp_buffer_hdr_t
*validate_buf(odp_buffer_t buf)
odp_buffer_hdr_t *buf_hdr;
handle.u32 = buf;
- /* For buffer handles, segment index must be 0 and
pool id
in range */
- if (handle.seg != 0 || handle.pool_id >=
ODP_CONFIG_BUFFER_POOLS)
It is erroneous to delete this check. The coverity warning is
a false positive that arises only in cases where
ODP_CONFIG_BUFFER_POOLS happens to be a power of two, which is
the case here but is not necessarily true.
Bill, I think you might be not correct. If I understand core right
then:
uint32_t pool_id:ODP_BUFFER_POOL_BITS;
#define ODP_CONFIG_BUFFER_POOLS 16
#define ODP_BUFFER_POOL_BITS ODP_BITSIZE(ODP_CONFIG_BUFFER_POOLS)
And this macro:
#define ODP_BITSIZE(x) \
((x) <= 2 ? 1 : \
((x) <= 4 ? 2 : \
((x) <= 8 ? 3 : \
((x) <= 16 ? 4 : \
((x) <= 32 ? 5 : \
((x) <= 64 ? 6 : \
((x) <= 128 ? 7 : \
((x) <= 256 ? 8 : \
((x) <= 512 ? 9 : \
((x) <= 1024 ? 10 : \
((x) <= 2048 ? 11 : \
((x) <= 4096 ? 12 : \
((x) <= 8196 ? 13 : \
((x) <= 16384 ? 14 : \
((x) <= 32768 ? 15 : \
((x) <= 65536 ? 16 : \
(0/0)))))))))))))))))
So ODP_CONFIG_BUFFER_POOLS must be power of two or it will fall
back to 0.
Maxim.
+ /* For buffer handles, segment index must be 0 */
+ if (handle.seg != 0)
return NULL;
pool_entry_t *pool =
odp_pool_to_entry(handle.pool_id);
--
1.8.5.1.163.gd7aced9
_______________________________________________
lng-odp mailing list
[email protected] <mailto:[email protected]>
<mailto:[email protected]
<mailto:[email protected]>>
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
