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.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
