On Mon, 2010-12-20 at 17:47 +0100, Takashi Iwai wrote:
> At Mon, 20 Dec 2010 16:27:49 +0000,
> Dimitris Papastamos wrote:
> >
> > On Mon, 2010-12-20 at 17:05 +0100, Takashi Iwai wrote:
> > > +static inline bool set_cache_val(void *base, unsigned int idx,
> > > + unsigned int val, unsigned int word_size)
> > > +{
> > > + if (word_size == 1) {
> > > + u8 *cache = base;
> > > + if (cache[idx] == val)
> > > + return true;
> > > + cache[idx] = val;
> > > + } else {
> > > + u16 *cache = base;
> > > + if (cache[idx] == val)
> > > + return true;
> > > + cache[idx] = val;
> > > + }
> > > + return false;
> > > +}
> >
> > If word_size is anything other than 1 byte, the above else will try to
> > handle it and assume it is 16 bits. I'd expect for an explicit check
> > for word_size == 2. A switch statement would perhaps be preferred for
> > legibility. It'd perhaps be wise to simply die via BUG() or similar if
> > an unsupported word size was passed in.
>
> Yes, this would be safer. I didn't put it since I wasn't sure whether
> BUG() content is also expanded at each caller. If yes, it would
> bloat. (Alternatively we may use snd_BUG_ON() -- it's built in only
> when CONFIG_SND_DEBUG is set.)
>
> Anyway, such a check should have been done rather at initialization.
I'd expect macro expansion to happen before inline function expansion.
I'll send in a patch to perform this check at init time once yours has
been merged.
Thanks,
Dimitrios
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
