[alsa-devel] [v4 10/12] ASoC: Intel: mrfld: add the DSP DAPM widgets

[Mark Brown]

On Thu, Aug 14, 2014 at 03:51:56PM +0530, Subhransu S. Prusty wrote:
> On Wed, Aug 13, 2014 at 09:14:54PM +0100, Mark Brown wrote:
> > On Mon, Aug 04, 2014 at 03:16:01PM +0530, Subhransu S. Prusty wrote:

> > > +	pr_debug("%s: widget = %s\n", __func__, w->name);
> > > +	for (i = 0; i < w->num_kcontrols; i++) {
> > > +		if (dapm_kcontrol_get_value(w->kcontrols[i])) {
> > > +			mc = (struct soc_mixer_control *)(w->kcontrols[i])->private_value;
> > > +			val |= 1 << mc->shift;
> > > +		}
> > > +	}

> > All my concerns about this still stand - using something called
> > _get_value() to do something other than read the control value (which
> > this does - it uses it as a boolean then does something else to read the
> > value) which doesn't seem good.

> I think this can be optimized. This need a bit of rework in the code, but
> need to check. But this would need an assumption that the controls are
> created in order.

> for (i = 0; i < w->num_kcontrols; i++) {
> 	if (dapm_kcontrol_get_value(w->kcontrols[i])) {
> 		val |= 1 << i;
> }

> Is this ok?

No, that doesn't seem safe.  Why not read the data from the control - if
we were calling a function which returned the value that'd be much
clearer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20140814/2a057d4b/attachment.sig>