[PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[Wei Yongjun]

From: Wei Yongjun <weiyongjun1@huawei.com>

Since we will remove items off the list using list_del() we need
to use a safe version of the list_for_each() macro aptly named
list_for_each_safe().

This is detected by Coccinelle semantic patch.

Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
DSP module bind")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 4 ++--
 sound/soc/intel/skylake/skl-topology.c | 4 ++--
 2 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 600faad..d43d197 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
 {
 	struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
 	struct skl *skl = ebus_to_skl(ebus);
-	struct skl_module_deferred_bind *modules;
+	struct skl_module_deferred_bind *modules, *tmp;
 
 	if (!list_empty(&skl->bind_list)) {
-		list_for_each_entry(modules, &skl->bind_list, node) {
+		list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
 			list_del(&modules->node);
 			kfree(modules);
 		}
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 74b3acf..aea7917 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
 	struct skl_module_cfg *src_module = NULL, *dst_module;
 	struct skl_sst *ctx = skl->skl_sst;
 	struct skl_pipe *s_pipe = mconfig->pipe;
-	struct skl_module_deferred_bind *modules;
+	struct skl_module_deferred_bind *modules, *tmp;
 
 	if (s_pipe->state == SKL_PIPE_INVALID)
 		return -EINVAL;
@@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
 
 		src_module = w_module->w->priv;
 
-		list_for_each_entry(modules, &skl->bind_list, node) {
+		list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
 			/*
 			 * When the destination module is deleted, Unbind the
 			 * modules from deferred bind list.

Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[Vinod Koul]

On Tue, Apr 25, 2017 at 03:28:17AM +0000, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
> 
> Since we will remove items off the list using list_del() we need
> to use a safe version of the list_for_each() macro aptly named
> list_for_each_safe().

ah yes, god catch

> This is detected by Coccinelle semantic patch.

It is a good practice to include the script you used, do you mind adding
that in the log?

> 
> Fixes: b8c722ddd548 ("ASoC: Intel: Skylake: Add support for deferred
> DSP module bind")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  sound/soc/intel/skylake/skl-pcm.c | 4 ++--
>  sound/soc/intel/skylake/skl-topology.c | 4 ++--
>  2 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
> index 600faad..d43d197 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -1323,10 +1323,10 @@ int skl_platform_unregister(struct device *dev)
>  {
>  	struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
>  	struct skl *skl = ebus_to_skl(ebus);
> -	struct skl_module_deferred_bind *modules;
> +	struct skl_module_deferred_bind *modules, *tmp;
>  
>  	if (!list_empty(&skl->bind_list)) {
> -		list_for_each_entry(modules, &skl->bind_list, node) {
> +		list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
>  			list_del(&modules->node);
>  			kfree(modules);
>  		}
> diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
> index 74b3acf..aea7917 100644
> --- a/sound/soc/intel/skylake/skl-topology.c
> +++ b/sound/soc/intel/skylake/skl-topology.c
> @@ -1091,7 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
>  	struct skl_module_cfg *src_module = NULL, *dst_module;
>  	struct skl_sst *ctx = skl->skl_sst;
>  	struct skl_pipe *s_pipe = mconfig->pipe;
> -	struct skl_module_deferred_bind *modules;
> +	struct skl_module_deferred_bind *modules, *tmp;
>  
>  	if (s_pipe->state == SKL_PIPE_INVALID)
>  		return -EINVAL;
> @@ -1105,7 +1105,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
>  
>  		src_module = w_module->w->priv;
>  
> -		list_for_each_entry(modules, &skl->bind_list, node) {
> +		list_for_each_entry_safe(modules, tmp, &skl->bind_list, node) {
>  			/*
>  			 * When the destination module is deleted, Unbind the
>  			 * modules from deferred bind list.
> 

-- 
~Vinod

RE: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[weiyongjun (A)]

> -----Original Message-----
> From: Vinod Koul [mailto:vinod.koul@intel.com]
> Sent: Tuesday, April 25, 2017 12:01 PM
> To: Wei Yongjun <weiyj.lk@gmail.com>
> Cc: Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; Jaroslav Kysela <perex@perex.cz>; Takashi Iwai
> <tiwai@suse.com>; Pardha Saradhi K
> <pardha.saradhi.kesapragada@intel.com>; G Kranthi
> <gudishax.kranthikumar@intel.com>; Hardik T Shah
> <hardik.t.shah@intel.com>; Jeeja KP <jeeja.kp@intel.com>; weiyongjun (A)
> <weiyongjun1@huawei.com>; alsa-devel@alsa-project.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use
> list_for_each_safe() when delete items
> 
> On Tue, Apr 25, 2017 at 03:28:17AM +0000, Wei Yongjun wrote:
> > From: Wei Yongjun <weiyongjun1@huawei.com>
> >
> > Since we will remove items off the list using list_del() we need
> > to use a safe version of the list_for_each() macro aptly named
> > list_for_each_safe().
> 
> ah yes, god catch
> 
> > This is detected by Coccinelle semantic patch.
> 
> It is a good practice to include the script you used, do you mind adding
> that in the log?
> 

Sure. 

@@
iterator name list_for_each;
expression E;
expression pos, head;
identifier l;
@@
*  list_for_each(pos, head)
{
...
*  \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
expression E;
expression pos, head, ep;
identifier list;
identifier l;
@@
*  list_for_each(pos, head)
{
...
* ep = list_entry(pos, ..., list);
...
*  \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_prev;
expression E;
expression pos, head;
identifier l;
@@
*  list_for_each_prev(pos, head)
{
...
*  \(list_del\|list_del_init\)(pos);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_entry;
expression E;
expression pos, head;
identifier l, member;
@@

*  list_for_each_entry(pos, head, member)
{
...
*  \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_entry_from;
expression E;
expression pos, head;
identifier l, member;
@@

*  list_for_each_entry_from(pos, head, member)
{
...
*  \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}

@@
iterator name list_for_each_entry_reverse;
expression E;
expression pos, head;
identifier l, member;
@@

*  list_for_each_entry_reverse(pos, head, member)
{
...
*  \(list_del\|list_del_init\)(&pos->member);
... when != \(goto l; \| break; \| return E; \| return;\)
}

Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[Vinod Koul]

On Tue, Apr 25, 2017 at 07:57:57AM +0000, weiyongjun (A) wrote:
> > >
> > > Since we will remove items off the list using list_del() we need
> > > to use a safe version of the list_for_each() macro aptly named
> > > list_for_each_safe().
> > 
> > ah yes, god catch
> > 
> > > This is detected by Coccinelle semantic patch.
> > 
> > It is a good practice to include the script you used, do you mind adding
> > that in the log?
> > 
> 
> Sure. 

Sorry I meant this should be in changelog of the patch :)

> 
> @@
> iterator name list_for_each;
> expression E;
> expression pos, head;
> identifier l;
> @@
> *  list_for_each(pos, head)
> {
> ...
> *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> expression E;
> expression pos, head, ep;
> identifier list;
> identifier l;
> @@
> *  list_for_each(pos, head)
> {
> ...
> * ep = list_entry(pos, ..., list);
> ...
> *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_prev;
> expression E;
> expression pos, head;
> identifier l;
> @@
> *  list_for_each_prev(pos, head)
> {
> ...
> *  \(list_del\|list_del_init\)(pos);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_entry;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
> 
> *  list_for_each_entry(pos, head, member)
> {
> ...
> *  \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_entry_from;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
> 
> *  list_for_each_entry_from(pos, head, member)
> {
> ...
> *  \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }
> 
> @@
> iterator name list_for_each_entry_reverse;
> expression E;
> expression pos, head;
> identifier l, member;
> @@
> 
> *  list_for_each_entry_reverse(pos, head, member)
> {
> ...
> *  \(list_del\|list_del_init\)(&pos->member);
> ... when != \(goto l; \| break; \| return E; \| return;\)
> }

-- 
~Vinod

RE: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[weiyongjun (A)]

> -----Original Message-----
> From: Vinod Koul [mailto:vinod.koul@intel.com]
> Sent: Wednesday, April 26, 2017 1:38 PM
> To: weiyongjun (A) <weiyongjun1@huawei.com>
> Cc: Wei Yongjun <weiyj.lk@gmail.com>; Liam Girdwood
> <lgirdwood@gmail.com>; Mark Brown <broonie@kernel.org>; Jaroslav
> Kysela <perex@perex.cz>; Takashi Iwai <tiwai@suse.com>; Pardha Saradhi K
> <pardha.saradhi.kesapragada@intel.com>; G Kranthi
> <gudishax.kranthikumar@intel.com>; Hardik T Shah
> <hardik.t.shah@intel.com>; Jeeja KP <jeeja.kp@intel.com>; alsa-
> devel@alsa-project.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH -next] ASoC: Intel: Skylake: Fix to use
> list_for_each_safe() when delete items
> 
> On Tue, Apr 25, 2017 at 07:57:57AM +0000, weiyongjun (A) wrote:
> > > >
> > > > Since we will remove items off the list using list_del() we need
> > > > to use a safe version of the list_for_each() macro aptly named
> > > > list_for_each_safe().
> > >
> > > ah yes, god catch
> > >
> > > > This is detected by Coccinelle semantic patch.
> > >
> > > It is a good practice to include the script you used, do you mind adding
> > > that in the log?
> > >
> >
> > Sure.
> 
> Sorry I meant this should be in changelog of the patch :)

OK, I will add this and send the v2 version patch later.

RE: [PATCH -next] ASoC: Intel: Skylake: Fix to use list_for_each_safe() when delete items

[weiyongjun (A)]

> On Tue, Apr 25, 2017 at 07:57:57AM +0000, weiyongjun (A) wrote:
> > > >
> > > > Since we will remove items off the list using list_del() we need
> > > > to use a safe version of the list_for_each() macro aptly named
> > > > list_for_each_safe().
> > >
> > > ah yes, god catch
> > >
> > > > This is detected by Coccinelle semantic patch.
> > >
> > > It is a good practice to include the script you used, do you mind adding
> > > that in the log?
> > >
> >
> > Sure.
> 
> Sorry I meant this should be in changelog of the patch :)


Oh, a similar patch has been applied. Please ignore this one.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=550b349af0f0e33fedb252aca8dc144299aca308

> 
> >
> > @@
> > iterator name list_for_each;
> > expression E;
> > expression pos, head;
> > identifier l;
> > @@
> > *  list_for_each(pos, head)
> > {
> > ...
> > *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(pos, ...);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > expression E;
> > expression pos, head, ep;
> > identifier list;
> > identifier l;
> > @@
> > *  list_for_each(pos, head)
> > {
> > ...
> > * ep = list_entry(pos, ..., list);
> > ...
> > *  \(list_del\|list_del_init\|list_move\|list_move_tail\)(&ep->list, ...);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_prev;
> > expression E;
> > expression pos, head;
> > identifier l;
> > @@
> > *  list_for_each_prev(pos, head)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(pos);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > *  list_for_each_entry(pos, head, member)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry_from;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > *  list_for_each_entry_from(pos, head, member)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> >
> > @@
> > iterator name list_for_each_entry_reverse;
> > expression E;
> > expression pos, head;
> > identifier l, member;
> > @@
> >
> > *  list_for_each_entry_reverse(pos, head, member)
> > {
> > ...
> > *  \(list_del\|list_del_init\)(&pos->member);
> > ... when != \(goto l; \| break; \| return E; \| return;\)
> > }
> 
> --
> ~Vinod