Re: [PATCH 07/14] Cleanup: avoid undefined shift operation

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@linux-foundation.org Signed-off-by: Dirk Hohndel dirk@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