| Message ID | 20170425032817.21959-1-weiyj.lk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. >
> -----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;\) }
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;\) > }
> -----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.
> 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
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.