Re: [alsa-devel] [PATCH 4/4] ASoC: clean up cache accesser

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.

+static inline unsigned int get_cache_val(const void *base, unsigned int idx,

• 			 unsigned int word_size)
  

+{

• if (word_size == 1) {

• const u8 *cache = base;
  

• return cache[idx];
  

• } else {

• const u16 *cache = base;
  

• return cache[idx];
  

• }

+}

Same here.

The rest looks good.

Thanks, Dimitrios