* [PATCH 2/3] ASoC: sprd: remove check of list iterator against head past the loop body
2022-03-31 21:50 [PATCH 1/3] ASoC: Intel: catpt: remove check of list iterator against head past the loop body Jakob Koschel
@ 2022-03-31 21:50 ` Jakob Koschel
2022-04-01 2:12 ` Baolin Wang
2022-03-31 21:50 ` [PATCH 3/3] ASoC: atmel-ssc: replace usage of found with dedicated list iterator variable Jakob Koschel
2022-04-01 7:00 ` [PATCH 1/3] ASoC: Intel: catpt: remove check of list iterator against head past the loop body Cezary Rojewski
2 siblings, 1 reply; 6+ messages in thread
From: Jakob Koschel @ 2022-03-31 21:50 UTC (permalink / raw)
To: Codrin Ciubotariu
Cc: Arnd Bergmann, Greg Kroah-Hartman, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Cezary Rojewski,
Pierre-Louis Bossart, Liam Girdwood, Jie Yang, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Orson Zhai, Baolin Wang,
Chunyan Zhang, Yang Yingliang, linux-arm-kernel, linux-kernel,
alsa-devel, Mike Rapoport, Brian Johannesmeyer,
Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel
When list_for_each_entry() completes the iteration over the whole list
without breaking the loop, the iterator value will be a bogus pointer
computed based on the head element.
While it is safe to use the pointer to determine if it was computed
based on the head element, either with list_entry_is_head() or
&pos->member == head, using the iterator variable after the loop should
be avoided.
In preparation to limiting the scope of a list iterator to the list
traversal loop, use a dedicated pointer to point to the found element [1].
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
sound/soc/sprd/sprd-mcdt.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/sound/soc/sprd/sprd-mcdt.c b/sound/soc/sprd/sprd-mcdt.c
index f6a55fa60c1b..6e27789a0df5 100644
--- a/sound/soc/sprd/sprd-mcdt.c
+++ b/sound/soc/sprd/sprd-mcdt.c
@@ -866,20 +866,19 @@ EXPORT_SYMBOL_GPL(sprd_mcdt_chan_dma_disable);
struct sprd_mcdt_chan *sprd_mcdt_request_chan(u8 channel,
enum sprd_mcdt_channel_type type)
{
- struct sprd_mcdt_chan *temp;
+ struct sprd_mcdt_chan *temp = NULL;
+ struct sprd_mcdt_chan *iter;
mutex_lock(&sprd_mcdt_list_mutex);
- list_for_each_entry(temp, &sprd_mcdt_chan_list, list) {
- if (temp->type == type && temp->id == channel) {
- list_del_init(&temp->list);
+ list_for_each_entry(iter, &sprd_mcdt_chan_list, list) {
+ if (iter->type == type && iter->id == channel) {
+ list_del_init(&iter->list);
+ temp = iter;
break;
}
}
- if (list_entry_is_head(temp, &sprd_mcdt_chan_list, list))
- temp = NULL;
-
mutex_unlock(&sprd_mcdt_list_mutex);
return temp;
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/3] ASoC: sprd: remove check of list iterator against head past the loop body
2022-03-31 21:50 ` [PATCH 2/3] ASoC: sprd: " Jakob Koschel
@ 2022-04-01 2:12 ` Baolin Wang
0 siblings, 0 replies; 6+ messages in thread
From: Baolin Wang @ 2022-04-01 2:12 UTC (permalink / raw)
To: Jakob Koschel
Cc: Codrin Ciubotariu, Arnd Bergmann, Greg Kroah-Hartman,
Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
Cezary Rojewski, Pierre-Louis Bossart, Liam Girdwood, Jie Yang,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Orson Zhai,
Chunyan Zhang, Yang Yingliang, linux-arm-kernel, LKML,
alsa-devel, Mike Rapoport, Brian Johannesmeyer,
Cristiano Giuffrida, Bos, H.J.
On Fri, Apr 1, 2022 at 5:52 AM Jakob Koschel <jakobkoschel@gmail.com> wrote:
>
> When list_for_each_entry() completes the iteration over the whole list
> without breaking the loop, the iterator value will be a bogus pointer
> computed based on the head element.
>
> While it is safe to use the pointer to determine if it was computed
> based on the head element, either with list_entry_is_head() or
> &pos->member == head, using the iterator variable after the loop should
> be avoided.
>
> In preparation to limiting the scope of a list iterator to the list
> traversal loop, use a dedicated pointer to point to the found element [1].
>
> Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
> Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
LGTM. Thanks.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> sound/soc/sprd/sprd-mcdt.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/sound/soc/sprd/sprd-mcdt.c b/sound/soc/sprd/sprd-mcdt.c
> index f6a55fa60c1b..6e27789a0df5 100644
> --- a/sound/soc/sprd/sprd-mcdt.c
> +++ b/sound/soc/sprd/sprd-mcdt.c
> @@ -866,20 +866,19 @@ EXPORT_SYMBOL_GPL(sprd_mcdt_chan_dma_disable);
> struct sprd_mcdt_chan *sprd_mcdt_request_chan(u8 channel,
> enum sprd_mcdt_channel_type type)
> {
> - struct sprd_mcdt_chan *temp;
> + struct sprd_mcdt_chan *temp = NULL;
> + struct sprd_mcdt_chan *iter;
>
> mutex_lock(&sprd_mcdt_list_mutex);
>
> - list_for_each_entry(temp, &sprd_mcdt_chan_list, list) {
> - if (temp->type == type && temp->id == channel) {
> - list_del_init(&temp->list);
> + list_for_each_entry(iter, &sprd_mcdt_chan_list, list) {
> + if (iter->type == type && iter->id == channel) {
> + list_del_init(&iter->list);
> + temp = iter;
> break;
> }
> }
>
> - if (list_entry_is_head(temp, &sprd_mcdt_chan_list, list))
> - temp = NULL;
> -
> mutex_unlock(&sprd_mcdt_list_mutex);
>
> return temp;
> --
> 2.25.1
>
--
Baolin Wang
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] ASoC: atmel-ssc: replace usage of found with dedicated list iterator variable
2022-03-31 21:50 [PATCH 1/3] ASoC: Intel: catpt: remove check of list iterator against head past the loop body Jakob Koschel
2022-03-31 21:50 ` [PATCH 2/3] ASoC: sprd: " Jakob Koschel
@ 2022-03-31 21:50 ` Jakob Koschel
2022-04-05 6:44 ` Codrin.Ciubotariu
2022-04-01 7:00 ` [PATCH 1/3] ASoC: Intel: catpt: remove check of list iterator against head past the loop body Cezary Rojewski
2 siblings, 1 reply; 6+ messages in thread
From: Jakob Koschel @ 2022-03-31 21:50 UTC (permalink / raw)
To: Codrin Ciubotariu
Cc: Arnd Bergmann, Greg Kroah-Hartman, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Cezary Rojewski,
Pierre-Louis Bossart, Liam Girdwood, Jie Yang, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Orson Zhai, Baolin Wang,
Chunyan Zhang, Yang Yingliang, linux-arm-kernel, linux-kernel,
alsa-devel, Mike Rapoport, Brian Johannesmeyer,
Cristiano Giuffrida, Bos, H.J.,
Jakob Koschel
To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.
To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].
This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
---
drivers/misc/atmel-ssc.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
index d6cd5537126c..343e25555fbb 100644
--- a/drivers/misc/atmel-ssc.c
+++ b/drivers/misc/atmel-ssc.c
@@ -25,25 +25,24 @@ static LIST_HEAD(ssc_list);
struct ssc_device *ssc_request(unsigned int ssc_num)
{
- int ssc_valid = 0;
- struct ssc_device *ssc;
+ struct ssc_device *ssc = NULL, *iter;
mutex_lock(&user_lock);
- list_for_each_entry(ssc, &ssc_list, list) {
- if (ssc->pdev->dev.of_node) {
- if (of_alias_get_id(ssc->pdev->dev.of_node, "ssc")
+ list_for_each_entry(iter, &ssc_list, list) {
+ if (iter->pdev->dev.of_node) {
+ if (of_alias_get_id(iter->pdev->dev.of_node, "ssc")
== ssc_num) {
- ssc->pdev->id = ssc_num;
- ssc_valid = 1;
+ iter->pdev->id = ssc_num;
+ ssc = iter;
break;
}
- } else if (ssc->pdev->id == ssc_num) {
- ssc_valid = 1;
+ } else if (iter->pdev->id == ssc_num) {
+ ssc = iter;
break;
}
}
- if (!ssc_valid) {
+ if (!ssc) {
mutex_unlock(&user_lock);
pr_err("ssc: ssc%d platform device is missing\n", ssc_num);
return ERR_PTR(-ENODEV);
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] ASoC: atmel-ssc: replace usage of found with dedicated list iterator variable
2022-03-31 21:50 ` [PATCH 3/3] ASoC: atmel-ssc: replace usage of found with dedicated list iterator variable Jakob Koschel
@ 2022-04-05 6:44 ` Codrin.Ciubotariu
0 siblings, 0 replies; 6+ messages in thread
From: Codrin.Ciubotariu @ 2022-04-05 6:44 UTC (permalink / raw)
To: jakobkoschel
Cc: arnd, gregkh, Nicolas.Ferre, alexandre.belloni, Claudiu.Beznea,
cezary.rojewski, pierre-louis.bossart, liam.r.girdwood, yang.jie,
broonie, perex, tiwai, orsonzhai, baolin.wang7, zhang.lyra,
yangyingliang, linux-arm-kernel, linux-kernel, alsa-devel, rppt,
bjohannesmeyer, c.giuffrida, h.j.bos
On 01.04.2022 00:50, Jakob Koschel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> To move the list iterator variable into the list_for_each_entry_*()
> macro in the future it should be avoided to use the list iterator
> variable after the loop body.
>
> To *never* use the list iterator variable after the loop it was
> concluded to use a separate iterator variable instead of a
> found boolean [1].
>
> This removes the need to use a found variable and simply checking if
> the variable was set, can determine if the break/goto was hit.
>
> Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
> Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Reviewed-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ASoC: Intel: catpt: remove check of list iterator against head past the loop body
2022-03-31 21:50 [PATCH 1/3] ASoC: Intel: catpt: remove check of list iterator against head past the loop body Jakob Koschel
2022-03-31 21:50 ` [PATCH 2/3] ASoC: sprd: " Jakob Koschel
2022-03-31 21:50 ` [PATCH 3/3] ASoC: atmel-ssc: replace usage of found with dedicated list iterator variable Jakob Koschel
@ 2022-04-01 7:00 ` Cezary Rojewski
2 siblings, 0 replies; 6+ messages in thread
From: Cezary Rojewski @ 2022-04-01 7:00 UTC (permalink / raw)
To: Jakob Koschel
Cc: Arnd Bergmann, Greg Kroah-Hartman, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Pierre-Louis Bossart,
Liam Girdwood, Jie Yang, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Orson Zhai, Baolin Wang, Chunyan Zhang,
Yang Yingliang, linux-arm-kernel, linux-kernel, alsa-devel,
Mike Rapoport, Brian Johannesmeyer, Cristiano Giuffrida, Bos,
H.J.,
Codrin Ciubotariu
On 2022-03-31 11:50 PM, Jakob Koschel wrote:
> When list_for_each_entry() completes the iteration over the whole list
> without breaking the loop, the iterator value will be a bogus pointer
> computed based on the head element.
>
> While it is safe to use the pointer to determine if it was computed
> based on the head element, either with list_entry_is_head() or
> &pos->member == head, using the iterator variable after the loop should
> be avoided.
>
> In preparation to limiting the scope of a list iterator to the list
> traversal loop, use a dedicated pointer to point to the found element [1].
>
> Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
> Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
Thank you for taking time and updating catpt-driver along the way.
Acked-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
> sound/soc/intel/catpt/pcm.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c
> index a26000cd5ceb..b103b2d4026e 100644
> --- a/sound/soc/intel/catpt/pcm.c
> +++ b/sound/soc/intel/catpt/pcm.c
> @@ -330,7 +330,8 @@ static int catpt_dai_apply_usettings(struct snd_soc_dai *dai,
> struct catpt_stream_runtime *stream)
> {
> struct snd_soc_component *component = dai->component;
> - struct snd_kcontrol *pos;
> + struct snd_kcontrol *pos = NULL;
> + struct snd_kcontrol *iter;
> struct catpt_dev *cdev = dev_get_drvdata(dai->dev);
> const char *name;
> int ret;
> @@ -354,12 +355,14 @@ static int catpt_dai_apply_usettings(struct snd_soc_dai *dai,
> return 0;
> }
>
> - list_for_each_entry(pos, &component->card->snd_card->controls, list) {
> - if (pos->private_data == component &&
> - !strncmp(name, pos->id.name, sizeof(pos->id.name)))
> + list_for_each_entry(iter, &component->card->snd_card->controls, list) {
> + if (iter->private_data == component &&
> + !strncmp(name, iter->id.name, sizeof(iter->id.name))) {
> + pos = iter;
> break;
> + }
> }
> - if (list_entry_is_head(pos, &component->card->snd_card->controls, list))
> + if (!pos)
> return -ENOENT;
>
> if (stream->template->type != CATPT_STRM_TYPE_LOOPBACK)
>
> base-commit: f82da161ea75dc4db21b2499e4b1facd36dab275
^ permalink raw reply [flat|nested] 6+ messages in thread