Bruno and I get defect reports from Coverity Scan for Gnulib. The most
recent one has this new complaint:
> *** CID 1588680: Incorrect _expression_ (ASSERT_SIDE_EFFECT)
> /gltests/test-byteswap.c: 43 in test_bswap_eval_once()
> 37
> 38 /* Test that the bswap functions evaluate their arguments once. */
> 39 static void
> 40 test_bswap_eval_once (void)
> 41 {
> 42 uint_least16_t value_1 = 0;
>>>> >>> CID 1588680: Incorrect _expression_ (ASSERT_SIDE_EFFECT)
>>>> >>> Argument "value_1++" of ASSERT() has a side effect. The containing function might work differently in a non-debug build.
> 43 ASSERT (bswap_16 (value_1++) == 0);
I checked the glibc sources, and long ago glibc indeed had a bug in this
area: bswap_16 and related function-like macros evaluated their
arguments more than once. This bug was fixed in the following glibc
commit dated Sat Aug 8 20:02:34 1998 +0000, and the fix appears in glibc
2.1 (1999).
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=7ce241a03e2c0b49482d9d05c8ddb765e89a01d9
Gnulib does not support glibc 2.1.x and older, so this should not be a
problem when porting to glibc. However, I worried that other platforms
might have the bug, until I noticed that m4/byteswap.m4 already
inadvertently tests for it. I installed the attached patch to document
the test.
We can ignore this Coverity defect report as a false positive, just as
we ignore many other defect reports.
I think this is a valid finding. It will operate one way in release builds (-DNDEBUG), and another way in debug builds (no NDEBUG).
When NDEBUG is in effect, the code `bswap_16 (value_1++) == 0` will be removed.
Maybe the test should be rewritten to something like:
uint16_t x = bswap_16 (value_1++);
ASSERT(x == 0);
Jeff