Skip to contentSkip to conversation

iio: imu: adis16400: Fix sign extension

PATCH In review Dan Carpenter 3 participants 5 messages Jan 30, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 iio: imu: adis16400: Fix sign extension

Because of C's type promotion rules, the code shifts in "((val16 & 0xFFF) << 4) >> 4;" cancel each other out and they're a no-op.

The intention here was to sign-extend then 11th bit so we can use the sign_extend32() function.

---
Please review this carefully. I'm pretty sure it's correct but this is the first time I've ever used the sign_extend32() function.

drivers/iio/imu/adis16400_core.c +1 -1
index b70873d..ed636c9 100644
@@ -414,7 +414,7 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
 		mutex_unlock(&indio_dev->mlock);
 		if (ret)
 			return ret;
-		val16 = ((val16 & 0xFFF) << 4) >> 4;
+		val16 = sign_extend32(val16, 11);
 		*val = val16;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
#1 lore raw
Quoted 11 lines · Dan Carpenter

Because of C's type promotion rules, the code shifts in "((val16 & 0xFFF) << 4) >> 4;" cancel each other out and they're a no-op.

The intention here was to sign-extend then 11th bit so we can use the sign_extend32() function.

---
Please review this carefully. I'm pretty sure it's correct but this is the first time I've ever used the sign_extend32() function.

Hi,

Rasmus already fixed this a couple of days ago.

Thanks,
- Lars

#2 lore raw
Quoted 13 lines · Lars-Peter Clausen
Quoted 11 lines · Dan Carpenter

Because of C's type promotion rules, the code shifts in "((val16 & 0xFFF) << 4) >> 4;" cancel each other out and they're a no-op.

The intention here was to sign-extend then 11th bit so we can use the sign_extend32() function.

---
Please review this carefully. I'm pretty sure it's correct but this is the first time I've ever used the sign_extend32() function.

Quoted 6 lines

Hi,

Rasmus already fixed this a couple of days ago.

Thanks,
- Lars

Oh. Good. I was CC'd on the other one that he fixed so I went looking for other bugs and I found this one.

Rasmus, are you using a static checker? Which one?

regards,
dan carpenter