On Fri, 28 May 2021 15:00:52 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> Hello Jonathan Cameron,
>
> The patch 25888dc51163: "staging:iio:sca3000 extract old event
> handling and move to poll for events from buffer" from May 18, 2011,
> leads to the following static checker warning:
>
> drivers/iio/accel/sca3000.c:734 sca3000_read_raw()
> warn: no-op. '((*val) << 19) >> 19'
>
> drivers/iio/accel/sca3000.c
> 709 static int sca3000_read_raw(struct iio_dev *indio_dev,
> 710 struct iio_chan_spec const *chan,
> 711 int *val,
> 712 int *val2,
> 713 long mask)
> 714 {
> 715 struct sca3000_state *st = iio_priv(indio_dev);
> 716 int ret;
> 717 u8 address;
> 718
> 719 switch (mask) {
> 720 case IIO_CHAN_INFO_RAW:
> 721 mutex_lock(&st->lock);
> 722 if (chan->type == IIO_ACCEL) {
> 723 if (st->mo_det_use_count) {
> 724 mutex_unlock(&st->lock);
> 725 return -EBUSY;
> 726 }
> 727 address = sca3000_addresses[chan->address][0];
> 728 ret = sca3000_read_data_short(st, address, 2);
> 729 if (ret < 0) {
> 730 mutex_unlock(&st->lock);
> 731 return ret;
> 732 }
> 733 *val = (be16_to_cpup((__be16 *)st->rx) >> 3) & 0x1FFF;
> 734 *val = ((*val) << (sizeof(*val) * 8 - 13)) >>
> ^^^^^^^^^^^^^^^^^^^^^^^
> 735 (sizeof(*val) * 8 - 13);
> ^^^^^^^^^^^^^^^^^^^^^^^
>
> This code works, but it relies on undefined behavior of left shift
> overflow and it's very unsatisfying. Pretty sure there is a UBSan
> warning for this at runtime.
Thanks Dan. Looks like a slightly odd variant on open coded sign_extend32()
Should be fine to replace with
*val = sign_extend32(*val, 13);
What can I say, I wrote this a long time ago when I was young and stupid :)
Anyhow, I'll roll a patch to do that shortly.
Jonathan
>
> 736 } else {
> 737 /* get the temperature when available */
> 738 ret = sca3000_read_data_short(st,
> 739 SCA3000_REG_TEMP_MSB_ADDR,
> 740 2);
> 741 if (ret < 0) {
> 742 mutex_unlock(&st->lock);
> 743 return ret;
> 744 }
> 745 *val = ((st->rx[0] & 0x3F) << 3) |
> 746 ((st->rx[1] & 0xE0) >> 5);
> 747 }
> 748 mutex_unlock(&st->lock);
> 749 return IIO_VAL_INT;
>
> regards,
> dan carpenter
