troydhanson / uthash

C macros for hash tables and more
Other
4.18k stars 926 forks source link

question about ut_hash compiler warning #195

Closed Jeff17Robbins closed 3 years ago

Jeff17Robbins commented 4 years ago

VS2019 is giving me this warning:

Warning C26451  Arithmetic overflow: Using operator '*' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '*' to avoid overflow (io.2).

I believe this calculation is the source:

2UL * (tbl)->num_buckets * sizeof(struct UT_hash_bucket)

num_buckets is a 32-bit unsigned. Is this a generic (cross-compiler) issue, or only a Microsoft issue? Should (tbl)->num_buckets be "up cast" before being multipled?

Quuxplusone commented 4 years ago

This looks like not quite a Microsoft issue, but a "32-bit long" issue. On platforms where long is 64 bits, there's no problem. (And, I mean, the problem is purely theoretical anyway.) We could use long long, i.e., 2uLL, to effectively guarantee 64 bits. However, uthash still wants to conform to C89, which didn't have the long long type at all.

Probably a simple fix would be to reverse the order of the operands:

sizeof(struct UT_hash_bucket) * (tbl)->num_buckets * 2

@Jeff17Robbins: if you do this locally, does the compiler warning disappear? If so, I'll push the fix to master.

Jeff17Robbins commented 4 years ago

@Quuxplusone I think the reversal did the trick. I say "I think", because, unfortunately, I'm using uthash.h in a big project and the circumstances under which the VS2019 "Error List" window gets refreshed with warnings are unclear to me. But I've only seen the warnings with the original code, not the reversed two lines. I am happy to test more tomorrow if you want to wait.

The two changes are shown in this diff:

diff --git "a/Source/MMS/RTXS/Py3/uthash.h" "b/Source/MMS/RTXS/Py3/uthash.h"
index a790ea59f..4dc2ab0bf 100644
--- "a/Source/MMS/RTXS/Py3/uthash.h"
+++ "b/Source/MMS/RTXS/Py3/uthash.h"
@@ -930,12 +930,12 @@ do {
   struct UT_hash_handle *_he_thh, *_he_hh_nxt;                                   \
   UT_hash_bucket *_he_new_buckets, *_he_newbkt;                                  \
   _he_new_buckets = (UT_hash_bucket*)uthash_malloc(                              \
-           2UL * (tbl)->num_buckets * sizeof(struct UT_hash_bucket));            \
+           sizeof(struct UT_hash_bucket) * (tbl)->num_buckets * 2);              \
   if (!_he_new_buckets) {                                                        \
     HASH_RECORD_OOM(oomed);                                                      \
   } else {                                                                       \
     uthash_bzero(_he_new_buckets,                                                \
-        2UL * (tbl)->num_buckets * sizeof(struct UT_hash_bucket));               \
+        sizeof(struct UT_hash_bucket) * (tbl)->num_buckets * 2);                 \
     (tbl)->ideal_chain_maxlen =                                                  \
        ((tbl)->num_items >> ((tbl)->log2_num_buckets+1U)) +                      \
        ((((tbl)->num_items & (((tbl)->num_buckets*2U)-1U)) != 0U) ? 1U : 0U);    \