| Message ID | 20220415122947.2754662-8-jakobkoschel@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Remove use of list iterator after loop body | expand |
On Fri, Apr 15, 2022 at 02:29:36PM +0200, Jakob Koschel wrote: > 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> > --- I absolutely hate the robotic commit message, but the change looks like it works, so: Reviewed-by: Vladimir Oltean <olteanv@gmail.com> > net/dsa/dsa.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c > index 89c6c86e746f..645522c4dd4a 100644 > --- a/net/dsa/dsa.c > +++ b/net/dsa/dsa.c > @@ -112,22 +112,21 @@ const struct dsa_device_ops *dsa_find_tagger_by_name(const char *buf) > > const struct dsa_device_ops *dsa_tag_driver_get(int tag_protocol) > { > - struct dsa_tag_driver *dsa_tag_driver; > + struct dsa_tag_driver *dsa_tag_driver = NULL, *iter; > const struct dsa_device_ops *ops; > - bool found = false; > > request_module("%s%d", DSA_TAG_DRIVER_ALIAS, tag_protocol); > > mutex_lock(&dsa_tag_drivers_lock); > - list_for_each_entry(dsa_tag_driver, &dsa_tag_drivers_list, list) { > - ops = dsa_tag_driver->ops; > + list_for_each_entry(iter, &dsa_tag_drivers_list, list) { > + ops = iter->ops; > if (ops->proto == tag_protocol) { > - found = true; > + dsa_tag_driver = iter; > break; > } > } > > - if (found) { > + if (dsa_tag_driver) { > if (!try_module_get(dsa_tag_driver->owner)) > ops = ERR_PTR(-ENOPROTOOPT); > } else { > -- > 2.25.1 >
On 4/15/2022 5:29 AM, Jakob Koschel wrote: > 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: Florian Fainelli <f.fainelli@gmail.com>
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index 89c6c86e746f..645522c4dd4a 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c @@ -112,22 +112,21 @@ const struct dsa_device_ops *dsa_find_tagger_by_name(const char *buf) const struct dsa_device_ops *dsa_tag_driver_get(int tag_protocol) { - struct dsa_tag_driver *dsa_tag_driver; + struct dsa_tag_driver *dsa_tag_driver = NULL, *iter; const struct dsa_device_ops *ops; - bool found = false; request_module("%s%d", DSA_TAG_DRIVER_ALIAS, tag_protocol); mutex_lock(&dsa_tag_drivers_lock); - list_for_each_entry(dsa_tag_driver, &dsa_tag_drivers_list, list) { - ops = dsa_tag_driver->ops; + list_for_each_entry(iter, &dsa_tag_drivers_list, list) { + ops = iter->ops; if (ops->proto == tag_protocol) { - found = true; + dsa_tag_driver = iter; break; } } - if (found) { + if (dsa_tag_driver) { if (!try_module_get(dsa_tag_driver->owner)) ops = ERR_PTR(-ENOPROTOOPT); } else {
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> --- net/dsa/dsa.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)