| Message ID | 20220407102900.3086255-4-jakobkoschel@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: Remove use of list iterator after loop body | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | warning | WARNING: line length of 81 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Hi Jakob, On Thu, Apr 07, 2022 at 12:28:48PM +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> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index 64f4fdd02902..f254f537c357 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1381,28 +1381,27 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port, > /* Mask of the local ports allowed to receive frames from a given fabric port */ > static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) > { > + struct dsa_port *dp = NULL, *iter, *other_dp; > struct dsa_switch *ds = chip->ds; > struct dsa_switch_tree *dst = ds->dst; > - struct dsa_port *dp, *other_dp; > - bool found = false; > u16 pvlan; > > /* dev is a physical switch */ > if (dev <= dst->last_switch) { > - list_for_each_entry(dp, &dst->ports, list) { > - if (dp->ds->index == dev && dp->index == port) { > - /* dp might be a DSA link or a user port, so it > + list_for_each_entry(iter, &dst->ports, list) { > + if (iter->ds->index == dev && iter->index == port) { > + /* iter might be a DSA link or a user port, so it > * might or might not have a bridge. > - * Use the "found" variable for both cases. > + * Set the "dp" variable for both cases. > */ > - found = true; > + dp = iter; > break; > } > } > /* dev is a virtual bridge */ > } else { > - list_for_each_entry(dp, &dst->ports, list) { > - unsigned int bridge_num = dsa_port_bridge_num_get(dp); > + list_for_each_entry(iter, &dst->ports, list) { > + unsigned int bridge_num = dsa_port_bridge_num_get(iter); > > if (!bridge_num) > continue; > @@ -1410,13 +1409,13 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) > if (bridge_num + dst->last_switch != dev) > continue; > > - found = true; > + dp = iter; > break; > } > } > > /* Prevent frames from unknown switch or virtual bridge */ > - if (!found) > + if (!dp) > return 0; > > /* Frames from DSA links and CPU ports can egress any local port */ > -- > 2.25.1 > Let's try to not make convoluted code worse. Do the following 2 patches achieve what you are looking for? Originally I had a single patch (what is now 2/2) but I figured it would be cleaner to break out the unrelated change into what is now 1/2. If you want I can submit these changes separately. -----------------------------[ cut here ]----------------------------- >From 2d84ecd87566b1535a04526b4ebb2764e764625f Mon Sep 17 00:00:00 2001 From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: Fri, 8 Apr 2022 15:15:30 +0300 Subject: [PATCH 1/2] net: dsa: mv88e6xxx: remove redundant check in mv88e6xxx_port_vlan() We know that "dev > dst->last_switch" in the "else" block. In other words, that "dev - dst->last_switch" is > 0. dsa_port_bridge_num_get(dp) can be 0, but the check "if (bridge_num + dst->last_switch != dev) continue", rewritten as "if (bridge_num != dev - dst->last_switch) continue", aka "if (bridge_num != something which cannot be 0) continue", makes it redundant to have the extra "if (!bridge_num) continue" logic, since a bridge_num of zero would have been skipped anyway. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/dsa/mv88e6xxx/chip.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 64f4fdd02902..b3aa0e5bc842 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1404,9 +1404,6 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) list_for_each_entry(dp, &dst->ports, list) { unsigned int bridge_num = dsa_port_bridge_num_get(dp); - if (!bridge_num) - continue; - if (bridge_num + dst->last_switch != dev) continue; -----------------------------[ cut here ]----------------------------- -----------------------------[ cut here ]----------------------------- >From dabafdbe38b408f7c563ad91fc6e57791055fed7 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: Fri, 8 Apr 2022 14:57:45 +0300 Subject: [PATCH 2/2] net: dsa: mv88e6xxx: refactor mv88e6xxx_port_vlan() To avoid bugs and speculative execution exploits due to type-confused pointers at the end of a list_for_each_entry() loop, one measure is to restrict code to not use the iterator variable outside the loop block. In the case of mv88e6xxx_port_vlan(), this isn't a problem, as we never let the loops exit through "natural causes" anyway, by using a "found" variable and then using the last "dp" iterator prior to the break, which is a safe thing to do. Nonetheless, with the expected new syntax, this pattern will no longer be possible. Profit off of the occasion and break the two port finding methods into smaller sub-functions. Somehow, returning a copy of the iterator pointer is still accepted. This change makes it redundant to have a "bool found", since the "dp" from mv88e6xxx_port_vlan() now holds NULL if we haven't found what we were looking for. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/dsa/mv88e6xxx/chip.c | 54 ++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index b3aa0e5bc842..1f35e89053e6 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1378,42 +1378,50 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port, return 0; } +static struct dsa_port *mv88e6xxx_find_port(struct dsa_switch_tree *dst, + int sw_index, int port) +{ + struct dsa_port *dp; + + list_for_each_entry(dp, &dst->ports, list) + if (dp->ds->index == sw_index && dp->index == port) + return dp; + + return NULL; +} + +static struct dsa_port * +mv88e6xxx_find_port_by_bridge_num(struct dsa_switch_tree *dst, + unsigned int bridge_num) +{ + struct dsa_port *dp; + + list_for_each_entry(dp, &dst->ports, list) + if (dsa_port_bridge_num_get(dp) == bridge_num) + return dp; + + return NULL; +} + /* Mask of the local ports allowed to receive frames from a given fabric port */ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) { struct dsa_switch *ds = chip->ds; struct dsa_switch_tree *dst = ds->dst; struct dsa_port *dp, *other_dp; - bool found = false; u16 pvlan; - /* dev is a physical switch */ if (dev <= dst->last_switch) { - list_for_each_entry(dp, &dst->ports, list) { - if (dp->ds->index == dev && dp->index == port) { - /* dp might be a DSA link or a user port, so it - * might or might not have a bridge. - * Use the "found" variable for both cases. - */ - found = true; - break; - } - } - /* dev is a virtual bridge */ + /* dev is a physical switch */ + dp = mv88e6xxx_find_port(dst, dev, port); } else { - list_for_each_entry(dp, &dst->ports, list) { - unsigned int bridge_num = dsa_port_bridge_num_get(dp); - - if (bridge_num + dst->last_switch != dev) - continue; - - found = true; - break; - } + /* dev is a virtual bridge */ + dp = mv88e6xxx_find_port_by_bridge_num(dst, + dev - dst->last_switch); } /* Prevent frames from unknown switch or virtual bridge */ - if (!found) + if (!dp) return 0; /* Frames from DSA links and CPU ports can egress any local port */ -----------------------------[ cut here ]-----------------------------
Hi Vladimir, > On 8. Apr 2022, at 14:31, Vladimir Oltean <olteanv@gmail.com> wrote: > > Hi Jakob, > > On Thu, Apr 07, 2022 at 12:28:48PM +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> >> --- >> drivers/net/dsa/mv88e6xxx/chip.c | 21 ++++++++++----------- >> 1 file changed, 10 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c >> index 64f4fdd02902..f254f537c357 100644 >> --- a/drivers/net/dsa/mv88e6xxx/chip.c >> +++ b/drivers/net/dsa/mv88e6xxx/chip.c >> @@ -1381,28 +1381,27 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port, >> /* Mask of the local ports allowed to receive frames from a given fabric port */ >> static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) >> { >> + struct dsa_port *dp = NULL, *iter, *other_dp; >> struct dsa_switch *ds = chip->ds; >> struct dsa_switch_tree *dst = ds->dst; >> - struct dsa_port *dp, *other_dp; >> - bool found = false; >> u16 pvlan; >> >> /* dev is a physical switch */ >> if (dev <= dst->last_switch) { >> - list_for_each_entry(dp, &dst->ports, list) { >> - if (dp->ds->index == dev && dp->index == port) { >> - /* dp might be a DSA link or a user port, so it >> + list_for_each_entry(iter, &dst->ports, list) { >> + if (iter->ds->index == dev && iter->index == port) { >> + /* iter might be a DSA link or a user port, so it >> * might or might not have a bridge. >> - * Use the "found" variable for both cases. >> + * Set the "dp" variable for both cases. >> */ >> - found = true; >> + dp = iter; >> break; >> } >> } >> /* dev is a virtual bridge */ >> } else { >> - list_for_each_entry(dp, &dst->ports, list) { >> - unsigned int bridge_num = dsa_port_bridge_num_get(dp); >> + list_for_each_entry(iter, &dst->ports, list) { >> + unsigned int bridge_num = dsa_port_bridge_num_get(iter); >> >> if (!bridge_num) >> continue; >> @@ -1410,13 +1409,13 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) >> if (bridge_num + dst->last_switch != dev) >> continue; >> >> - found = true; >> + dp = iter; >> break; >> } >> } >> >> /* Prevent frames from unknown switch or virtual bridge */ >> - if (!found) >> + if (!dp) >> return 0; >> >> /* Frames from DSA links and CPU ports can egress any local port */ >> -- >> 2.25.1 >> > > Let's try to not make convoluted code worse. Do the following 2 patches > achieve what you are looking for? Originally I had a single patch (what > is now 2/2) but I figured it would be cleaner to break out the unrelated > change into what is now 1/2. I do agree with not making convoluted code worse, but I was reluctant with e.g. introducing new functions for this because others essentially have the opposite opinion on this. I however like solving it that way, it makes it a lot cleaner. > > If you want I can submit these changes separately. Sure if you want to submit them separately, go ahead. Otherwise I can integrate it into a v2, whatever you prefer essentially. > > -----------------------------[ cut here ]----------------------------- > From 2d84ecd87566b1535a04526b4ebb2764e764625f Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Fri, 8 Apr 2022 15:15:30 +0300 > Subject: [PATCH 1/2] net: dsa: mv88e6xxx: remove redundant check in > mv88e6xxx_port_vlan() > > We know that "dev > dst->last_switch" in the "else" block. > In other words, that "dev - dst->last_switch" is > 0. > > dsa_port_bridge_num_get(dp) can be 0, but the check > "if (bridge_num + dst->last_switch != dev) continue", rewritten as > "if (bridge_num != dev - dst->last_switch) continue", aka > "if (bridge_num != something which cannot be 0) continue", > makes it redundant to have the extra "if (!bridge_num) continue" logic, > since a bridge_num of zero would have been skipped anyway. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index 64f4fdd02902..b3aa0e5bc842 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1404,9 +1404,6 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) > list_for_each_entry(dp, &dst->ports, list) { > unsigned int bridge_num = dsa_port_bridge_num_get(dp); > > - if (!bridge_num) > - continue; > - > if (bridge_num + dst->last_switch != dev) > continue; > > -----------------------------[ cut here ]----------------------------- > > -----------------------------[ cut here ]----------------------------- > From dabafdbe38b408f7c563ad91fc6e57791055fed7 Mon Sep 17 00:00:00 2001 > From: Vladimir Oltean <vladimir.oltean@nxp.com> > Date: Fri, 8 Apr 2022 14:57:45 +0300 > Subject: [PATCH 2/2] net: dsa: mv88e6xxx: refactor mv88e6xxx_port_vlan() > > To avoid bugs and speculative execution exploits due to type-confused > pointers at the end of a list_for_each_entry() loop, one measure is to > restrict code to not use the iterator variable outside the loop block. > > In the case of mv88e6xxx_port_vlan(), this isn't a problem, as we never > let the loops exit through "natural causes" anyway, by using a "found" > variable and then using the last "dp" iterator prior to the break, which > is a safe thing to do. > > Nonetheless, with the expected new syntax, this pattern will no longer > be possible. > > Profit off of the occasion and break the two port finding methods into > smaller sub-functions. Somehow, returning a copy of the iterator pointer > is still accepted. > > This change makes it redundant to have a "bool found", since the "dp" > from mv88e6xxx_port_vlan() now holds NULL if we haven't found what we > were looking for. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 54 ++++++++++++++++++-------------- > 1 file changed, 31 insertions(+), 23 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index b3aa0e5bc842..1f35e89053e6 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1378,42 +1378,50 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port, > return 0; > } > > +static struct dsa_port *mv88e6xxx_find_port(struct dsa_switch_tree *dst, > + int sw_index, int port) > +{ > + struct dsa_port *dp; > + > + list_for_each_entry(dp, &dst->ports, list) > + if (dp->ds->index == sw_index && dp->index == port) > + return dp; > + > + return NULL; > +} > + > +static struct dsa_port * > +mv88e6xxx_find_port_by_bridge_num(struct dsa_switch_tree *dst, > + unsigned int bridge_num) > +{ > + struct dsa_port *dp; > + > + list_for_each_entry(dp, &dst->ports, list) > + if (dsa_port_bridge_num_get(dp) == bridge_num) > + return dp; > + > + return NULL; > +} > + > /* Mask of the local ports allowed to receive frames from a given fabric port */ > static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) > { > struct dsa_switch *ds = chip->ds; > struct dsa_switch_tree *dst = ds->dst; > struct dsa_port *dp, *other_dp; > - bool found = false; > u16 pvlan; > > - /* dev is a physical switch */ > if (dev <= dst->last_switch) { > - list_for_each_entry(dp, &dst->ports, list) { > - if (dp->ds->index == dev && dp->index == port) { > - /* dp might be a DSA link or a user port, so it > - * might or might not have a bridge. > - * Use the "found" variable for both cases. > - */ > - found = true; > - break; > - } > - } > - /* dev is a virtual bridge */ > + /* dev is a physical switch */ > + dp = mv88e6xxx_find_port(dst, dev, port); > } else { > - list_for_each_entry(dp, &dst->ports, list) { > - unsigned int bridge_num = dsa_port_bridge_num_get(dp); > - > - if (bridge_num + dst->last_switch != dev) > - continue; > - > - found = true; > - break; > - } > + /* dev is a virtual bridge */ > + dp = mv88e6xxx_find_port_by_bridge_num(dst, > + dev - dst->last_switch); > } > > /* Prevent frames from unknown switch or virtual bridge */ > - if (!found) > + if (!dp) > return 0; > > /* Frames from DSA links and CPU ports can egress any local port */ > -----------------------------[ cut here ]----------------------------- Thanks, Jakob
On Sat, Apr 09, 2022 at 01:44:00AM +0200, Jakob Koschel wrote: > > Let's try to not make convoluted code worse. Do the following 2 patches > > achieve what you are looking for? Originally I had a single patch (what > > is now 2/2) but I figured it would be cleaner to break out the unrelated > > change into what is now 1/2. > > I do agree with not making convoluted code worse, but I was reluctant with > e.g. introducing new functions for this because others essentially > have the opposite opinion on this. > > I however like solving it that way, it makes it a lot cleaner. Yeah, I think 'just adapt to the context and style and intentions of the code you're changing and don't try to push a robotic one-size-fits-all solution' is sensible enough for an initial guiding principle. > > If you want I can submit these changes separately. > > Sure if you want to submit them separately, go ahead. Otherwise I can > integrate it into a v2, whatever you prefer essentially. If you're moving quickly feel free to pick them up. I have lots of other things on my backlog so it won't be until late next week until I even consider submitting these.
> On 9. Apr 2022, at 01:50, Vladimir Oltean <olteanv@gmail.com> wrote: > > On Sat, Apr 09, 2022 at 01:44:00AM +0200, Jakob Koschel wrote: >>> Let's try to not make convoluted code worse. Do the following 2 patches >>> achieve what you are looking for? Originally I had a single patch (what >>> is now 2/2) but I figured it would be cleaner to break out the unrelated >>> change into what is now 1/2. >> >> I do agree with not making convoluted code worse, but I was reluctant with >> e.g. introducing new functions for this because others essentially >> have the opposite opinion on this. >> >> I however like solving it that way, it makes it a lot cleaner. > > Yeah, I think 'just adapt to the context and style and intentions of the > code you're changing and don't try to push a robotic one-size-fits-all > solution' is sensible enough for an initial guiding principle. > >>> If you want I can submit these changes separately. >> >> Sure if you want to submit them separately, go ahead. Otherwise I can >> integrate it into a v2, whatever you prefer essentially. > > If you're moving quickly feel free to pick them up. I have lots of other > things on my backlog so it won't be until late next week until I even > consider submitting these. I'm planning to send a v2 earlier than that, so I'll just integrate it there. Thanks, Jakob
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 64f4fdd02902..f254f537c357 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1381,28 +1381,27 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port, /* Mask of the local ports allowed to receive frames from a given fabric port */ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) { + struct dsa_port *dp = NULL, *iter, *other_dp; struct dsa_switch *ds = chip->ds; struct dsa_switch_tree *dst = ds->dst; - struct dsa_port *dp, *other_dp; - bool found = false; u16 pvlan; /* dev is a physical switch */ if (dev <= dst->last_switch) { - list_for_each_entry(dp, &dst->ports, list) { - if (dp->ds->index == dev && dp->index == port) { - /* dp might be a DSA link or a user port, so it + list_for_each_entry(iter, &dst->ports, list) { + if (iter->ds->index == dev && iter->index == port) { + /* iter might be a DSA link or a user port, so it * might or might not have a bridge. - * Use the "found" variable for both cases. + * Set the "dp" variable for both cases. */ - found = true; + dp = iter; break; } } /* dev is a virtual bridge */ } else { - list_for_each_entry(dp, &dst->ports, list) { - unsigned int bridge_num = dsa_port_bridge_num_get(dp); + list_for_each_entry(iter, &dst->ports, list) { + unsigned int bridge_num = dsa_port_bridge_num_get(iter); if (!bridge_num) continue; @@ -1410,13 +1409,13 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int port) if (bridge_num + dst->last_switch != dev) continue; - found = true; + dp = iter; break; } } /* Prevent frames from unknown switch or virtual bridge */ - if (!found) + if (!dp) return 0; /* Frames from DSA links and CPU ports can egress any local port */
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/net/dsa/mv88e6xxx/chip.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)