diff mbox series

[V2] iio: adc: ina2xx: add in early -EINVAL returns in case statements

Message ID 20181016161418.29213-1-colin.king@canonical.com (mailing list archive)
State New, archived
Headers show
Series [V2] iio: adc: ina2xx: add in early -EINVAL returns in case statements | expand

Commit Message

Colin King Oct. 16, 2018, 4:14 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

Static analysis with CoverityScan is throwing warnings that specific
case statements are missing breaks.  Rather than adding breaks, add
return -EINVAL to the specific case statements to clarify the
error return paths. Fix also saves 50 bytes.

Before:
   text	   data	    bss	    dec	    hex	filename
  21418	   4936	    128	  26482	   6772	drivers/iio/adc/ina2xx-adc.o

After:
  dec	    hex	filename
  21370	   4936	    128	  26434	   6742	drivers/iio/adc/ina2xx-adc.o

(gcc 8.2, x86-64)

Detected by CoverityScan, CID#1462408 ("Missing break in switch")

---

V2: use returns instead of break statements to keep with the
current style used in the switch statement.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/iio/adc/ina2xx-adc.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Stefan Brüns Oct. 16, 2018, 6:26 p.m. UTC | #1
On Dienstag, 16. Oktober 2018 18:14:18 CEST Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Static analysis with CoverityScan is throwing warnings that specific
> case statements are missing breaks.  Rather than adding breaks, add
> return -EINVAL to the specific case statements to clarify the
> error return paths. Fix also saves 50 bytes.
> 
> Before:
>    text	   data	    bss	    dec	    hex	filename
>   21418	   4936	    128	  26482	   6772	drivers/iio/adc/ina2xx-adc.o
> 
> After:
>   dec	    hex	filename
>   21370	   4936	    128	  26434	   6742	drivers/iio/adc/ina2xx-adc.o
> 
> (gcc 8.2, x86-64)
> 
> Detected by CoverityScan, CID#1462408 ("Missing break in switch")
> 
> ---
> 
> V2: use returns instead of break statements to keep with the
> current style used in the switch statement.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Reviewed-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

> ---
>  drivers/iio/adc/ina2xx-adc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> index d1239624187d..bdd7cba6f6b0 100644
> --- a/drivers/iio/adc/ina2xx-adc.c
> +++ b/drivers/iio/adc/ina2xx-adc.c
> @@ -250,6 +250,7 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
>  			*val2 = chip->shunt_resistor_uohm;
>  			return IIO_VAL_FRACTIONAL;
>  		}
> +		return -EINVAL;
> 
>  	case IIO_CHAN_INFO_HARDWAREGAIN:
>  		switch (chan->address) {
> @@ -262,6 +263,7 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
>  			*val = chip->range_vbus == 32 ? 1 : 2;
>  			return IIO_VAL_INT;
>  		}
> +		return -EINVAL;
>  	}
> 
>  	return -EINVAL;
Jonathan Cameron Oct. 21, 2018, 2:45 p.m. UTC | #2
On Tue, 16 Oct 2018 20:26:57 +0200
Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote:

> On Dienstag, 16. Oktober 2018 18:14:18 CEST Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> > 
> > Static analysis with CoverityScan is throwing warnings that specific
> > case statements are missing breaks.  Rather than adding breaks, add
> > return -EINVAL to the specific case statements to clarify the
> > error return paths. Fix also saves 50 bytes.
> > 
> > Before:
> >    text	   data	    bss	    dec	    hex	filename
> >   21418	   4936	    128	  26482	   6772	drivers/iio/adc/ina2xx-adc.o
> > 
> > After:
> >   dec	    hex	filename
> >   21370	   4936	    128	  26434	   6742	drivers/iio/adc/ina2xx-adc.o
> > 
> > (gcc 8.2, x86-64)
> > 
> > Detected by CoverityScan, CID#1462408 ("Missing break in switch")
> > 
> > ---
> > 
> > V2: use returns instead of break statements to keep with the
> > current style used in the switch statement.
> > 
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>  
> 
> Reviewed-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>

Thanks, and applied to the togreg branch of iio.git and pushed
out as testing for the autobuilders to play with it.

Note however that I was confused when there appeared to be no sign off
in the patch from Colin after applying.  It ended up below an
additional cut line so git am dropped it.

All fixed up now.

Thanks,

Jonathan

> 
> > ---
> >  drivers/iio/adc/ina2xx-adc.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
> > index d1239624187d..bdd7cba6f6b0 100644
> > --- a/drivers/iio/adc/ina2xx-adc.c
> > +++ b/drivers/iio/adc/ina2xx-adc.c
> > @@ -250,6 +250,7 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
> >  			*val2 = chip->shunt_resistor_uohm;
> >  			return IIO_VAL_FRACTIONAL;
> >  		}
> > +		return -EINVAL;
> > 
> >  	case IIO_CHAN_INFO_HARDWAREGAIN:
> >  		switch (chan->address) {
> > @@ -262,6 +263,7 @@ static int ina2xx_read_raw(struct iio_dev *indio_dev,
> >  			*val = chip->range_vbus == 32 ? 1 : 2;
> >  			return IIO_VAL_INT;
> >  		}
> > +		return -EINVAL;
> >  	}
> > 
> >  	return -EINVAL;  
> 
>
diff mbox series

Patch

diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index d1239624187d..bdd7cba6f6b0 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -250,6 +250,7 @@  static int ina2xx_read_raw(struct iio_dev *indio_dev,
 			*val2 = chip->shunt_resistor_uohm;
 			return IIO_VAL_FRACTIONAL;
 		}
+		return -EINVAL;
 
 	case IIO_CHAN_INFO_HARDWAREGAIN:
 		switch (chan->address) {
@@ -262,6 +263,7 @@  static int ina2xx_read_raw(struct iio_dev *indio_dev,
 			*val = chip->range_vbus == 32 ? 1 : 2;
 			return IIO_VAL_INT;
 		}
+		return -EINVAL;
 	}
 
 	return -EINVAL;