Hi Bruno,

Am Mo., 5. Apr. 2021 um 15:02 Uhr schrieb Bruno Haible <br...@clisp.org>:
> Hi Marc,
>
> > * tests/test-hamt.c: New file.
>
> Can you please have a quick look whether these Coverity findings (from our
> weekly Coverity run) are relevant?
>

Coverity seems to be a good tool. I haven't yet tested GCC's new static
analyzer.


>
> ________________________________________________________________________________________________________
> *** CID 1503613:  Null pointer dereferences  (REVERSE_INULL)
> /gllib/hamt.c: 876 in bucket_do_while()
> 870       size_t cnt = 0;
> 871       size_t elt_count = bucket->elt_count;
> 872       Hamt_entry *const *elts = bucket->elts;
> 873       for (size_t i = 0; i < elt_count; ++i)
> 874         {
> 875           *success = proc (elts[i], data);
> >>>     CID 1503613:  Null pointer dereferences  (REVERSE_INULL)
> >>>     Null-checking "success" suggests that it may be null, but it has
> already been dereferenced on all paths leading to the check.
> 876           if (!success)
> 877             return cnt;
> 878           ++cnt;
> 879         }
> 880       return cnt;
> 881     }
>

Correct finding. It should be "!*success".


>
> ________________________________________________________________________________________________________
> *** CID 1503612:  Uninitialized variables  (UNINIT)
> /gllib/hamt.c: 952 in hamt_iterator()
> 946       Hamt_iterator iter;
> 947       iter.hamt = hamt_copy (hamt);
> 948       Hamt_entry *entry = hamt->root;
> 949       if (entry == NULL)
> 950         {
> 951           iter.depth = -1;
> >>>     CID 1503612:  Uninitialized variables  (UNINIT)
> >>>     Using uninitialized value "iter". Field "iter.path" is
> uninitialized.
> 952           return iter;
> 953         }
> 954       iter.depth = 0;
> 955       iter.path = 0;
> 956       iter.position = 0;
> 957       while (iter.entry[iter.depth] = entry, entry_type (entry) ==
> subtrie_entry)
>
>
Irrelevant, but I guess the warning can be silenced by moving up the
initialization of "iter.path" and "iter.position".


>
> ________________________________________________________________________________________________________
> *** CID 1503618:  Incorrect expression  (PW.ASSIGN_WHERE_COMPARE_MEANT)
> /gltests/test-hamt.c: 155 in ()
> 149       ASSERT (hamt_do_while (hamt2, proc, &flag) == 4);
> 150       ASSERT (sum == 52);
> 151
> 152       hamt1 = hamt_remove (hamt2, &x4);
> 153       sum = 0;
> 154       ASSERT (hamt_do_while (hamt2, proc, &flag) == 4);
> >>>     CID 1503618:  Incorrect expression  (PW.ASSIGN_WHERE_COMPARE_MEANT)
> >>>     use of "=" where "==" may have been intended
> 155       ASSERT (sum = 52);
> 156       sum = 0;
> 157       ASSERT (hamt_do_while (hamt1, proc, &flag) == 3);
> 158       ASSERT (sum  = 48);
> 159
> 160       hamt_free (hamt1);
>
>
Correct observation.


>
> ________________________________________________________________________________________________________
> *** CID 1503615:  Incorrect expression  (PW.ASSIGN_WHERE_COMPARE_MEANT)
> /gltests/test-hamt.c: 158 in ()
> 152       hamt1 = hamt_remove (hamt2, &x4);
> 153       sum = 0;
> 154       ASSERT (hamt_do_while (hamt2, proc, &flag) == 4);
> 155       ASSERT (sum = 52);
> 156       sum = 0;
> 157       ASSERT (hamt_do_while (hamt1, proc, &flag) == 3);
> >>>     CID 1503615:  Incorrect expression  (PW.ASSIGN_WHERE_COMPARE_MEANT)
> >>>     use of "=" where "==" may have been intended
> 158       ASSERT (sum  = 48);
> 159
> 160       hamt_free (hamt1);
> 161       hamt_free (hamt2);
> 162       free_element (y5);
> 163     }
>

Dito.


>
> ________________________________________________________________________________________________________
> *** CID 1503614:    (ASSERT_SIDE_EFFECT)
> /gltests/test-hamt.c: 158 in test_general()
> 152       hamt1 = hamt_remove (hamt2, &x4);
> 153       sum = 0;
> 154       ASSERT (hamt_do_while (hamt2, proc, &flag) == 4);
> 155       ASSERT (sum = 52);
> 156       sum = 0;
> 157       ASSERT (hamt_do_while (hamt1, proc, &flag) == 3);
> >>>     CID 1503614:    (ASSERT_SIDE_EFFECT)
> >>>     Assignment "sum = 48" has a side effect.  This code will work
> differently in a non-debug build.
> 158       ASSERT (sum  = 48);
> 159
> 160       hamt_free (hamt1);
> 161       hamt_free (hamt2);
> 162       free_element (y5);
> 163     }
> /gltests/test-hamt.c: 155 in test_general()
> 149       ASSERT (hamt_do_while (hamt2, proc, &flag) == 4);
> 150       ASSERT (sum == 52);
> 151
> 152       hamt1 = hamt_remove (hamt2, &x4);
> 153       sum = 0;
> 154       ASSERT (hamt_do_while (hamt2, proc, &flag) == 4);
> >>>     CID 1503614:    (ASSERT_SIDE_EFFECT)
> >>>     Assignment "sum = 52" has a side effect.  This code will work
> differently in a non-debug build.
> 155       ASSERT (sum = 52);
> 156       sum = 0;
> 157       ASSERT (hamt_do_while (hamt1, proc, &flag) == 3);
> 158       ASSERT (sum  = 48);
> 159
> 160       hamt_free (hamt1);
>
>
These are aftereffects.

I will push the fixes.

Thanks,

Marc

Reply via email to