[PATCH 07/14] Cleanup: avoid undefined shift operation
Jef Driesen
jef at libdivecomputer.org
Thu Jan 4 05:45:10 PST 2018
On 2018-01-03 20:35, Dirk Hohndel wrote:
> Shifting a 32bit value by 32 is undefined.
> Instead of using shifts to create the mask, explicitly create it by
> subtracting 1 from the signbit value (and using bitwise NOT to fill all
> the higher bits).
>
> Coverity CID 207769
>
> Suggested-by: Linus Torvalds <torvalds at linux-foundation.org>
> Signed-off-by: Dirk Hohndel <dirk at hohndel.org>
> ---
> src/uwatec_smart_parser.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/uwatec_smart_parser.c b/src/uwatec_smart_parser.c
> index a70acb7b91c6..053cad9f2914 100644
> --- a/src/uwatec_smart_parser.c
> +++ b/src/uwatec_smart_parser.c
> @@ -886,15 +886,14 @@ uwatec_smart_fixsignbit (unsigned int x, unsigned
> int n)
> return 0;
>
> unsigned int signbit = (1 << (n - 1));
> - unsigned int mask = (0xFFFFFFFF << n);
>
> // When turning a two's-complement number with a certain number
> // of bits into one with more bits, the sign bit must be repeated
> // in all the extra bits.
> if ((x & signbit) == signbit)
> - return x | mask;
> + return x | ~(signbit - 1);
> else
> - return x & ~mask;
> + return x & (signbit - 1);
> }
I would have kept the mask variable, like this:
unsigned int mask = (signbit - 1);
if ((x & signbit) == signbit)
return x | ~mask;
else
return x & mask;
But the current version is fine too.
Jef
More information about the devel
mailing list