Messages in this thread | | | From | Xiaomeng Tong <> | Subject | Re: [PATCH] opp: use list iterator only inside the loop | Date | Thu, 31 Mar 2022 10:20:09 +0800 |
| |
On Thu, 31 Mar 2022 07:42:35 +0530, Viresh Kumar wrote:
> Hi Xiaomeng, > > On 31-03-22, 09:58, Xiaomeng Tong wrote: > > dev = new_dev->dev; > > Why is this added here ?
Sorry for that. I will delete it in next patch.
> > > > > As discussed before, > > Please remember that whatever you write here will go in the commit > logs for ever and no one will ever know what you discussed and with > whom. > > This area should describe the problem at hand. >
ok, thank you for the suggestion.
> > we should avoid to use a list iterator variable > > outside the loop which is considered harmful[1]. > > > > In this case, use a new variable 'iter' as the list iterator, while > > use the old variable 'new_dev' as a dedicated pointer to point to the > > found entry. > > > > [1]: https://lkml.org/lkml/2022/2/17/1032 > > > > Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com> > > --- > > drivers/opp/debugfs.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c > > index 596c185b5dda..a4476985e4ce 100644 > > --- a/drivers/opp/debugfs.c > > +++ b/drivers/opp/debugfs.c > > @@ -187,14 +187,19 @@ void opp_debug_register(struct opp_device *opp_dev, struct opp_table *opp_table) > > static void opp_migrate_dentry(struct opp_device *opp_dev, > > struct opp_table *opp_table) > > { > > - struct opp_device *new_dev; > > + struct opp_device *new_dev = NULL, *iter; > > const struct device *dev; > > struct dentry *dentry; > > > > /* Look for next opp-dev */ > > - list_for_each_entry(new_dev, &opp_table->dev_list, node) > > - if (new_dev != opp_dev) > > + list_for_each_entry(iter, &opp_table->dev_list, node) > > + if (iter != opp_dev) { > > + new_dev = iter; > > break; > > + } > > + > > + if (!new_dev) > > I will rather make this BUG_ON(!new_dev); >
Ok, i will take it.
> > + return; > > > > /* new_dev is guaranteed to be valid here */ > > dev = new_dev->dev;
-- Xiaomeng Tong
| |