Messages in this thread | | | From | Stefan Brüns <> | Subject | Re: [PATCH] iio: adc: ina2xx: fix missing break statement | Date | Wed, 10 Oct 2018 12:42:39 +0200 |
| |
On Montag, 8. Oktober 2018 23:09:04 CEST Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The IIO_CHAN_INFO_SCALE case is missing a break statement and in > the unlikely event that chan->address is not matched in the nested > switch statement then the code falls through to the following > IIO_CHAN_INFO_HARDWAREGAIN case. Fix this by adding the missing > break. While we are fixing this, it's probably a good idea to > add in a break statement to the IIO_CHAN_INFO_HARDWAREGAIN case > too (this is a moot point). > > Detected by CoverityScan, CID#1462408 ("Missing break in switch")
Although it is good for code clarity to add a break statement, the code can never return anything but -EINVAL in case chan->address is not handled in IIO_CHAN_INFO_SCALE:
----- switch (mask) { case IIO_CHAN_INFO_SCALE: switch (chan->address) { case INA2XX_SHUNT_VOLTAGE: ... return IIO_VAL_FRACTIONAL; case INA2XX_BUS_VOLTAGE: ... return IIO_VAL_FRACTIONAL;
case INA2XX_CURRENT: ... return IIO_VAL_FRACTIONAL;
case INA2XX_POWER: ... return IIO_VAL_FRACTIONAL; }
case IIO_CHAN_INFO_HARDWAREGAIN: switch (chan->address) { case INA2XX_SHUNT_VOLTAGE: ... return IIO_VAL_FRACTIONAL; case INA2XX_BUS_VOLTAGE: ... return IIO_VAL_INT; } } return -EINVAL; -----
The addresses handled in INFO_HARDWAREGAIN is a subset of the ones in INFO_SCALE.
I would prefer an early "return -EINVAL" here, as it matches better with the other "switch (mask)" cases above.
Kind regards,
Stefan
-- Stefan Brüns / Bergstraße 21 / 52062 Aachen home: +49 241 53809034 mobile: +49 151 50412019[unhandled content-type:application/pgp-signature] | |