[alsa-devel] [PATCH] ALSA: control: Simplify snd_ctl_elem_list() implementation

[Takashi Iwai]

On Mon, 22 May 2017 21:55:02 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On May 23 2017 00:52, Takashi Iwai wrote:
> > This patch simplifies the code of snd_ctl_elem_list() in the following
> > ways:
> >
> > - Avoid a vmalloc() temporary buffer but do copy in each iteration;
> >    the vmalloc buffer was introduced at the time we took the spinlock
> >    for the ctl element management.
> >
> > - Use the standard list_for_each_entry() macro
> >
> > - Merge two loops into one;
> >    it used to be a loop for skipping until offset becomes zero and
> >    another loop to copy the data.  They can be folded into a single
> >    loop easily.
> >
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> 
> Reviewed-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
> Tested-by: Takashi Sakamoto <o-takashi at sakamocchi.jp>
> 
> I have one comment.
> 
> > ---
> >   sound/core/control.c | 66 +++++++++++++++++++---------------------------------
> >   1 file changed, 24 insertions(+), 42 deletions(-)
> >
> > diff --git a/sound/core/control.c b/sound/core/control.c
> > index c109b82eef4b..47080da8451a 100644
> > --- a/sound/core/control.c
> > +++ b/sound/core/control.c
> > @@ -747,11 +747,11 @@ static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
> >   static int snd_ctl_elem_list(struct snd_card *card,
> >   			     struct snd_ctl_elem_list __user *_list)
> >   {
> > -	struct list_head *plist;
> >   	struct snd_ctl_elem_list list;
> >   	struct snd_kcontrol *kctl;
> > -	struct snd_ctl_elem_id *dst, *id;
> > +	struct snd_ctl_elem_id id;
> >   	unsigned int offset, space, jidx;
> > +	int err = 0;
> >   	
> >   	if (copy_from_user(&list, _list, sizeof(list)))
> >   		return -EFAULT;
> > @@ -760,52 +760,34 @@ static int snd_ctl_elem_list(struct snd_card *card,
> >   	/* try limit maximum space */
> >   	if (space > 16384)
> >   		return -ENOMEM;
> 
> We can get rid of this limitation because allocation in kernel space
> is not performed anymore.

Yes, likely.


thanks,

Takashi