Re: [PATCH v2 2/5] drxd: Fix warning caused by new entries in an enum

[Steve Kerrison <steve@stevekerrison.com>]

Hi Andreas,

> I'd prefer returning -EINVAL for unsupported parameters.
>
> [snip]
> 
> I already had a patch for this, but forgot to submit it together with
> the frontend.h bits.

That seems reasonable. Do I need to do anything with this? I'm happy for
Mauro to scrub my drxd and mxl patches and use yours instead.

> Btw., "status = status;" looks odd.

Heh, yes it does. I wonder if that was put in to deal with an "unused
variable" compiler warning before the switch statement had a default
case? Otherwise, perhaps it's from the department of redundancy
department.

Regards,
-- 
Steve Kerrison MEng Hons.
http://www.stevekerrison.com/ 

On Mon, 2011-05-09 at 00:10 +0200, Andreas Oberritter wrote:
> On 05/08/2011 09:17 PM, Steve Kerrison wrote:
> > Additional bandwidth modes have been added in frontend.h
> > drxd_hard.c had no default case so the compiler was warning about
> > a non-exhausive switch statement.
> > 
> > This has been fixed by making the default behaviour the same as
> > BANDWIDTH_AUTO, with the addition of a printk to notify if this
> > ever happens.
> > 
> > Signed-off-by: Steve Kerrison <steve@stevekerrison.com>
> > ---
> >  drivers/media/dvb/frontends/drxd_hard.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/dvb/frontends/drxd_hard.c b/drivers/media/dvb/frontends/drxd_hard.c
> > index 30a78af..b3b0704 100644
> > --- a/drivers/media/dvb/frontends/drxd_hard.c
> > +++ b/drivers/media/dvb/frontends/drxd_hard.c
> > @@ -2325,6 +2325,10 @@ static int DRX_Start(struct drxd_state *state, s32 off)
> >  		   InitEC and ResetEC
> >  		   functions */
> >  		switch (p->bandwidth) {
> > +		default:
> > +			printk(KERN_INFO "drxd: Unsupported bandwidth mode %u, reverting to default\n",
> > +				p->bandwidth);
> > +			/* Fall back to auto */
> 
> I'd prefer returning -EINVAL for unsupported parameters.
> 
> >  		case BANDWIDTH_AUTO:
> >  		case BANDWIDTH_8_MHZ:
> >  			/* (64/7)*(8/8)*1000000 */
> 
> I already had a patch for this, but forgot to submit it together with the frontend.h bits.
> 
> From 73d630b57f584d7e35cac5e27149cbc564aedde2 Mon Sep 17 00:00:00 2001
> From: Andreas Oberritter <obi@linuxtv.org>
> Date: Fri, 8 Apr 2011 16:39:20 +0000
> Subject: [PATCH 2/2] DVB: drxd_hard: handle new bandwidths by returning -EINVAL
> 
> Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
> ---
>  drivers/media/dvb/frontends/drxd_hard.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/dvb/frontends/drxd_hard.c b/drivers/media/dvb/frontends/drxd_hard.c
> index 30a78af..53319f4 100644
> --- a/drivers/media/dvb/frontends/drxd_hard.c
> +++ b/drivers/media/dvb/frontends/drxd_hard.c
> @@ -2348,6 +2348,9 @@ static int DRX_Start(struct drxd_state *state, s32 off)
>  			status = Write16(state,
>  					 FE_AG_REG_IND_DEL__A, 71, 0x0000);
>  			break;
> +		default:
> +			status = -EINVAL;
> +			break;
>  		}
>  		status = status;
>  		if (status < 0)