From: Julia Lawall <[email protected]>
It is not clear whether the assignment should be part of the if branch
suggested by its indentation. The patch preserves the current semantics.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r disable braces4@
position p1,p2;
statement S1,S2;
@@
(
if (...) { ... }
|
if (...) S1@p1 S2@p2
)
@script:python@
p1 << r.p1;
p2 << r.p2;
@@
if (p1[0].column == p2[0].column):
cocci.print_main("branch",p1)
cocci.print_secs("after",p2)
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/infiniband/hw/nes/nes_hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/nes/nes_hw.c b/drivers/infiniband/hw/nes/nes_hw.c
index f41d890..9a379d4 100644
--- a/drivers/infiniband/hw/nes/nes_hw.c
+++ b/drivers/infiniband/hw/nes/nes_hw.c
@@ -2740,7 +2740,7 @@ void nes_nic_ce_handler(struct nes_device *nesdev, struct nes_hw_nic_cq *cq)
/* restart the queue if it had been stopped */
if (netif_queue_stopped(nesvnic->netdev))
netif_wake_queue(nesvnic->netdev);
- sq_cqes = 0;
+ sq_cqes = 0;
}
} else {
rqes_processed ++;
> It is not clear whether the assignment should be part of the if branch
> suggested by its indentation. The patch preserves the current semantics.
Thanks. Chien/Faisal -- are the current semantics correct or should we
add braces { } to match the indentation? From a quick look at the code,
I think the patch we actually want is:
drivers/infiniband/hw/nes/nes_hw.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/nes/nes_hw.c b/drivers/infiniband/hw/nes/nes_hw.c
index 07c4004..f8233c8 100644
--- a/drivers/infiniband/hw/nes/nes_hw.c
+++ b/drivers/infiniband/hw/nes/nes_hw.c
@@ -2737,9 +2737,9 @@ void nes_nic_ce_handler(struct nes_device *nesdev, struct nes_hw_nic_cq *cq)
nesnic->sq_tail &= nesnic->sq_size-1;
if (sq_cqes > 128) {
barrier();
- /* restart the queue if it had been stopped */
- if (netif_queue_stopped(nesvnic->netdev))
- netif_wake_queue(nesvnic->netdev);
+ /* restart the queue if it had been stopped */
+ if (netif_queue_stopped(nesvnic->netdev))
+ netif_wake_queue(nesvnic->netdev);
sq_cqes = 0;
}
} else {
--
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
I think the white space is meant to look like this. I did look at
whether the "sq_cqes = 0;" should only be done if netif_queue_stopped().
In the end I decided this was what was intended, but it would be
better if someone more familiar with the code reviewed it.
Reported-by: Julia Lawall <[email protected]>
Signed-off-by: Dan Carpenter <[email protected]>
---
v2: changed different indents
diff --git a/drivers/infiniband/hw/nes/nes_hw.c b/drivers/infiniband/hw/nes/nes_hw.c
index 57874a1..293977b 100644
--- a/drivers/infiniband/hw/nes/nes_hw.c
+++ b/drivers/infiniband/hw/nes/nes_hw.c
@@ -2737,9 +2737,9 @@ void nes_nic_ce_handler(struct nes_device *nesdev, struct nes_hw_nic_cq *cq)
nesnic->sq_tail &= nesnic->sq_size-1;
if (sq_cqes > 128) {
barrier();
- /* restart the queue if it had been stopped */
- if (netif_queue_stopped(nesvnic->netdev))
- netif_wake_queue(nesvnic->netdev);
+ /* restart the queue if it had been stopped */
+ if (netif_queue_stopped(nesvnic->netdev))
+ netif_wake_queue(nesvnic->netdev);
sq_cqes = 0;
}
} else {
> I think the white space is meant to look like this. I did look at
> whether the "sq_cqes = 0;" should only be done if netif_queue_stopped().
> In the end I decided this was what was intended, but it would be
> better if someone more familiar with the code reviewed it.
>
> Reported-by: Julia Lawall <[email protected]>
> Signed-off-by: Dan Carpenter <[email protected]>
> ---
> v2: changed different indents
Looks to me. Thanks for the patch.
Acked-by: Chien Tung <[email protected]>
Chien