Continue at the bottom of a loop are removed.
Issue found using drop_continue.cocci Coccinelle script.
Signed-off-by: Arushi Singhal <[email protected]>
---
Changes in v2:
- Braces is dropped from if with single statement.
drivers/net/ethernet/amd/ni65.c | 4 +---
drivers/net/ethernet/neterion/s2io.c | 4 +---
drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 4 +---
3 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/amd/ni65.c b/drivers/net/ethernet/amd/ni65.c
index e248d1a..8931ce6 100644
--- a/drivers/net/ethernet/amd/ni65.c
+++ b/drivers/net/ethernet/amd/ni65.c
@@ -435,10 +435,8 @@ static int __init ni65_probe1(struct net_device *dev,int ioaddr)
}
if(cards[i].vendor_id) {
for(j=0;j<3;j++)
- if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j]) {
+ if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
release_region(ioaddr, cards[i].total_size);
- continue;
- }
}
break;
}
diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index b8983e7..4738bc7 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -3679,11 +3679,9 @@ static void restore_xmsi_data(struct s2io_nic *nic)
writeq(nic->msix_info[i].data, &bar0->xmsi_data);
val64 = (s2BIT(7) | s2BIT(15) | vBIT(msix_index, 26, 6));
writeq(val64, &bar0->xmsi_access);
- if (wait_for_msix_trans(nic, msix_index)) {
+ if (wait_for_msix_trans(nic, msix_index))
DBG_PRINT(ERR_DBG, "%s: index: %d failed\n",
__func__, msix_index);
- continue;
- }
}
}
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 15fa47f..5cd4f3f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -258,10 +258,8 @@ nfp_net_pf_alloc_vnics(struct nfp_pf *pf, void __iomem *ctrl_bar,
ctrl_bar += NFP_PF_CSR_SLICE_SIZE;
/* Kill the vNIC if app init marked it as invalid */
- if (nn->port && nn->port->type == NFP_PORT_INVALID) {
+ if (nn->port && nn->port->type == NFP_PORT_INVALID)
nfp_net_pf_free_vnic(pf, nn);
- continue;
- }
}
if (list_empty(&pf->vnics))
--
2.7.4
On Sat, 3 Mar 2018 21:44:56 +0530, Arushi Singhal wrote:
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> index 15fa47f..5cd4f3f 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> @@ -258,10 +258,8 @@ nfp_net_pf_alloc_vnics(struct nfp_pf *pf, void __iomem *ctrl_bar,
> ctrl_bar += NFP_PF_CSR_SLICE_SIZE;
>
> /* Kill the vNIC if app init marked it as invalid */
> - if (nn->port && nn->port->type == NFP_PORT_INVALID) {
> + if (nn->port && nn->port->type == NFP_PORT_INVALID)
> nfp_net_pf_free_vnic(pf, nn);
> - continue;
> - }
This is an error handling path so the continue makes sense here to
indicate the processing can't ever fall through if more statements are
ever added to the loop. But OK.
> }
>
> if (list_empty(&pf->vnics))
From: Arushi Singhal <[email protected]>
Date: Sat, 3 Mar 2018 21:44:56 +0530
> Continue at the bottom of a loop are removed.
> Issue found using drop_continue.cocci Coccinelle script.
>
> Signed-off-by: Arushi Singhal <[email protected]>
> ---
> Changes in v2:
> - Braces is dropped from if with single statement.
Sorry there is no way I am applying this.
Just blindly removing continue statements that some static checker
or compiler warning mentions is completely unacceptable.
Actually _LOOK_ at this code:
> if(cards[i].vendor_id) {
> for(j=0;j<3;j++)
> - if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j]) {
> + if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
> release_region(ioaddr, cards[i].total_size);
> - continue;
> - }
> }
The extraneous continue statement is the _least_ of the problems here.
This code, if the if() statement triggers, will release the ioaddr
region and then _CONTINUE_ the for(j) loop, and try to access the
registers using the same 'ioaddr' again.
That's actually a _REAL_ bug, and you're making it seem like
the behavior is intentional by editing it like this.
And the bug probably exists because this entire sequence has
misaligned closing curly braces. It makes it look like
the continue is for the outer loop, which would be fine.
But it's not. It's for the inner loop, and this causes the "use
ioaddr after releasing it" bug.
Please do not submit patches like this one, it makes for a lot of
wasted auditing and review for people. The onus is on you to read
and understand the code you are editing before submitting your
changes.
Thank you.
From: Arushi Singhal <[email protected]>
Date: Wed, 7 Mar 2018 22:22:13 +0530
> Yes I know it's for the inner loop.
> But I am not able to find, where I am wrong here
You are making what is essentially a nop change to a piece of code,
which if you had instead read carefully would have instead inspired
you to fix a real genuine bug instead.