On Tue, 4 Mar 2025 09:22:43 +0300 Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > head: cd3215bbcb9d4321def93fea6cfad4d5b42b9d1d > commit: 949abd1ca5a4342d9fb8c774035222e65dd1cfe4 [3206/7719] iio: adc: ad4030: add averaging support > config: openrisc-randconfig-r071-20250304 (https://download.01.org/0day-ci/archive/20250304/202503040954.n6MhjSsV-lkp@xxxxxxxxx/config) > compiler: or1k-linux-gcc (GCC) 14.2.0 > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@xxxxxxxxx> > | Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > | Closes: https://lore.kernel.org/r/202503040954.n6MhjSsV-lkp@xxxxxxxxx/ > > smatch warnings: > drivers/iio/adc/ad4030.c:378 ad4030_get_chan_scale() error: 'scan_type' dereferencing possible ERR_PTR() > > vim +/scan_type +378 drivers/iio/adc/ad4030.c > > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 366 static int ad4030_get_chan_scale(struct iio_dev *indio_dev, > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 367 struct iio_chan_spec const *chan, > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 368 int *val, > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 369 int *val2) > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 370 { > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 371 struct ad4030_state *st = iio_priv(indio_dev); > 949abd1ca5a4342 Esteban Blanc 2025-02-14 372 const struct iio_scan_type *scan_type; > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 373 > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 374 if (chan->differential) { > 949abd1ca5a4342 Esteban Blanc 2025-02-14 375 scan_type = iio_get_current_scan_type(indio_dev, > 949abd1ca5a4342 Esteban Blanc 2025-02-14 376 st->chip->channels); > > > Do we need an if (IS_ERR(scan_type)) return;? No but it is a bit of chase to establish why not so I think it would be sensible from the point of view of resilience and readability to add that check. That function can fail with an ERR_PTR() if either the callback it uses returns an error (the one in this driver never does) or the value is out of range (I assume with only 2 values we manage to avoid that too though I didn't actually check). It's not a high performance path to read this so the check would be a good addition but I'm not going to rush in the change. Esteban, would you mind spinning a patch to add the check if you agree with the reasoning? +CC linux-iio for wider visibility. Thanks, Jonathan > > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 377 *val = (st->vref_uv * 2) / MILLI; > 949abd1ca5a4342 Esteban Blanc 2025-02-14 @378 *val2 = scan_type->realbits; > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 379 return IIO_VAL_FRACTIONAL_LOG2; > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 380 } > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 381 > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 382 *val = st->vref_uv / MILLI; > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 383 *val2 = chan->scan_type.realbits; > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 384 return IIO_VAL_FRACTIONAL_LOG2; > 0cb8b324852f3e3 Esteban Blanc 2025-02-14 385 } >
