Re: [RFC v2 2/6] ALSA: pcm: add IEC958 channel status control helper

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Fri, Jan 22, 2016 at 06:48:28PM +0100, Arnaud Pouliquen wrote:
> +static int snd_pcm_iec958_get(struct snd_kcontrol *kcontrol,
> +			      struct snd_ctl_elem_value *uctl)
> +{
> +	struct snd_pcm_iec958_params *params = snd_kcontrol_chip(kcontrol);
> +
> +	if (params->mutex)
> +		mutex_unlock(params->mutex);
> +	uctl->value.iec958.status[0] = params->iec->status[0];
> +	uctl->value.iec958.status[1] = params->iec->status[1];
> +	uctl->value.iec958.status[2] = params->iec->status[2];
> +	uctl->value.iec958.status[3] = params->iec->status[3];
> +	uctl->value.iec958.status[4] = params->iec->status[4];
> +	if (params->mutex)
> +		mutex_unlock(params->mutex);

I thought I had already commented about the mutex - maybe not.  Please
don't make these conditional like this: if you need the mutex to
eliminate a race condition, then you need it to eliminate the race
condition and it can't be "optional".  Elimination of race conditions
is never optional!

Please get rid of all these conditions and make it mandatory that a
mutex is supplied.

Thanks.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.