Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i

[Takashi Iwai <tiwai@suse.de>]

At Mon, 22 Apr 2013 13:29:41 +0200 (CEST),
Eldad Zack wrote:
> 
> 
> 
> On Mon, 22 Apr 2013, Takashi Iwai wrote:
> 
> > At Mon, 22 Apr 2013 01:44:04 +0200,
> > Eldad Zack wrote:
> > > 
> > > Replace the usage of SNDRV_PCM_FORMAT_* macros with the equivalent
> > > SNDRV_PCM_FMTBIT_* macros, and the result be can assigned directly
> > > to the formats field.
> > 
> > Note that SNDRV_PCM_FMTBIT_* are 64bit integer.  And it makes sense to
> > use FMTBIT only when multiple formats are supposed.  For a single
> > format, keeping SNDRV_PCM_FORMAT_* is more reasonable.
> > 
> > In other words, a proper fix would be to replace the type of variable
> > format with snd_pcm_format_t, and cast at converting to format bits
> > like
> > 	1ULL << (unsigned int)pcm_format
> 
> I see your point. I just figured making the assignment directly would
> make sense just for this case.
> 
> There are a couple of other places with the same conversion that 
> "break" the strong typing - how about something like this:
> 
> 	#define pcm_format_to_bits(fmt) (1uLL << ((__force int)(fmt)))
> 
> Or an equivalent function?

Yep, I just wanted to propose that :)


Takashi

> 
> Cheers,
> Eldad
> 
> > > Modify the variable name to reflect the change.
> > > 
> > > The following sparse messages are silenced:
> > > 
> > > sound/usb/format.c:377:44: warning: incorrect type in assignment (different base types)
> > > sound/usb/format.c:377:44:    expected int [signed] pcm_format
> > > sound/usb/format.c:377:44:    got restricted snd_pcm_format_t [usertype] <noident>
> > > sound/usb/format.c:379:44: warning: incorrect type in assignment (different base types)
> > > sound/usb/format.c:379:44:    expected int [signed] pcm_format
> > > sound/usb/format.c:379:44:    got restricted snd_pcm_format_t [usertype] <noident>
> > > sound/usb/format.c:382:36: warning: incorrect type in assignment (different base types)
> > > sound/usb/format.c:382:36:    expected int [signed] pcm_format
> > > sound/usb/format.c:382:36:    got restricted snd_pcm_format_t [usertype] <noident>
> > > 
> > > Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> > > ---
> > >  sound/usb/format.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/sound/usb/format.c b/sound/usb/format.c
> > > index 020ede0..e025e28 100644
> > > --- a/sound/usb/format.c
> > > +++ b/sound/usb/format.c
> > > @@ -365,7 +365,7 @@ static int parse_audio_format_i(struct snd_usb_audio *chip,
> > >  {
> > >  	struct usb_interface_descriptor *altsd = get_iface_desc(iface);
> > >  	int protocol = altsd->bInterfaceProtocol;
> > > -	int pcm_format, ret;
> > > +	int pcm_formats, ret;
> > >  
> > >  	if (fmt->bFormatType == UAC_FORMAT_TYPE_III) {
> > >  		/* FIXME: the format type is really IECxxx
> > > @@ -377,14 +377,14 @@ static int parse_audio_format_i(struct snd_usb_audio *chip,
> > >  		case USB_ID(0x0763, 0x2003): /* M-Audio Audiophile USB */
> > >  			if (chip->setup == 0x00 && 
> > >  			    fp->altsetting == 6)
> > > -				pcm_format = SNDRV_PCM_FORMAT_S16_BE;
> > > +				pcm_formats = SNDRV_PCM_FMTBIT_S16_BE;
> > >  			else
> > > -				pcm_format = SNDRV_PCM_FORMAT_S16_LE;
> > > +				pcm_formats = SNDRV_PCM_FMTBIT_S16_LE;
> > >  			break;
> > >  		default:
> > > -			pcm_format = SNDRV_PCM_FORMAT_S16_LE;
> > > +			pcm_formats = SNDRV_PCM_FMTBIT_S16_LE;
> > >  		}
> > > -		fp->formats = 1uLL << pcm_format;
> > > +		fp->formats = pcm_formats;
> > >  	} else {
> > >  		fp->formats = parse_audio_format_i_type(chip, fp, format,
> > >  							fmt, protocol);
> > > -- 
> > > 1.8.1.5
> > > 
> > 
>