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

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.