On Sun, Apr 10, 2022 at 12:51:56PM +0200, Jakob Koschel wrote:
> I've just looked at this again in a bit more detail while integrating it into
> the patch series.
>
> I realized that this just shifts the 'problem' to using the 'pos' iterator
> variable after the loop.
> If the scope of the list iterator would be lowered to the list traversal loop
> it would also make sense
> to also do it for list_for_each().
Yes, but list_for_each() was never formulated as being problematic in
the same way as list_for_each_entry(), was it? I guess I'm starting to
not understand what is the true purpose of the changes.
> What do you think about doing it this way:
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c
> b/drivers/net/dsa/sja1105/sja1105_vl.c
> index b7e95d60a6e4..f5b0502c1098 100644
> --- a/drivers/net/dsa/sja1105/sja1105_vl.c
> +++ b/drivers/net/dsa/sja1105/sja1105_vl.c
> @@ -28,6 +28,7 @@ static int sja1105_insert_gate_entry(struct
> sja1105_gating_config *gating_cfg,
> list_add(&e->list, &gating_cfg->entries);
> } else {
> struct sja1105_gate_entry *p;
> + struct list_head *pos = NULL;
>
> list_for_each_entry(p, &gating_cfg->entries, list) {
> if (p->interval == e->interval) {
> @@ -37,10 +38,14 @@ static int sja1105_insert_gate_entry(struct
> sja1105_gating_config *gating_cfg,
> goto err;
> }
>
> - if (e->interval < p->interval)
> + if (e->interval < p->interval) {
> + pos = &p->list;
> break;
> + }
> }
> - list_add(&e->list, p->list.prev);
> + if (!pos)
> + pos = &gating_cfg->entries;
> + list_add(&e->list, pos->prev);
> }
>
> gating_cfg->num_entries++;
> --
>
> >
> > Thanks for the suggestion.
> >
> >> }
> >>
> >> gating_cfg->num_entries++;
> >> -----------------------------[ cut here ]-----------------------------
> >
> > [1]
> > https://lore.kernel.org/linux-kernel/20220407102900.3086255-12-jakobkosc...@gmail.com/
> >
> > Jakob
>
> Thanks,
> Jakob
