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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


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

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel


[ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

Add to Google Powered by Linux