[alsa-devel] [PATCH 6/9] ASoC: max98095: Add missing negative channel checks

Mark Brown broonie at kernel.org
Wed Oct 30 22:31:21 CET 2013


On Wed, Oct 30, 2013 at 06:36:21PM +0100, Takashi Iwai wrote:
> Mark Brown wrote:
> > On Wed, Oct 30, 2013 at 08:35:04AM +0100, Takashi Iwai wrote:

> > > +	if (channel < 0)
> > > +		return channel;
> > >  	BUG_ON(channel > 1);

> > This doesn't seem to fit in well with the adjacent code - the handling
> > of out of bounds data isn't consistent though it looks like they're both
> > doing essentially the same thing here.

> Actually BUG_ON(channel > 1) is a pretty stupid line and should be
> removed.  You'll see that this never happens if you read the code.

Yeah, but then nor does a return of less than zero...

> Overall, this driver has way too many BUG_ON() without
> considerations.  Such BUT_ON()'s are worse than nothing, IMO.

Yeah, I'm ambivalent.  I think they're not too bad if it's about a
fairly small part of the driver agreeing with itself which is the case
here.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20131030/7bc5c055/attachment.sig>


More information about the Alsa-devel mailing list