[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