Skip to contentSkip to conversation

brcmfmac: testing the wrong variable in brcmf_rx_hdrpull()

PATCH Applied Dan Carpenter 3 participants 3 messages Apr 19, 2016 – Apr 26, 2016

Subsystems (2)
Kernel janitors Broadcom BRCM80211 IEEE802.11 wireless drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 brcmfmac: testing the wrong variable in brcmf_rx_hdrpull()

Smatch complains about this code:

drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c:335 brcmf_rx_hdrpull()
error: we previously assumed '*ifp' could be null (see line 333)

The problem is that we recently changed these from "ifp" to "*ifp" but there was one that we didn't update.

-       if (ret || !ifp || !ifp->ndev) {
+       if (ret || !(*ifp) || !(*ifp)->ndev) {
                if (ret != -ENODATA && ifp)
                                       ^^^

- ifp->stats.rx_errors++;
+ (*ifp)->stats.rx_errors++;

I have updated it to *ifp as well. We always call this function is a non-NULL "ifp" pointer, btw.

Fixes c462ebcdfe42 ('brcmfmac: create common function for handling brcmf_proto_hdrpull()')
Signed-off-by Dan Carpenter <[email protected]>
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +1 -1
index 1b476d1..b590499 100644
@@ -331,7 +331,7 @@ static int brcmf_rx_hdrpull(struct brcmf_pub *drvr, struct sk_buff *skb,
 	ret = brcmf_proto_hdrpull(drvr, true, skb, ifp);
 
 	if (ret || !(*ifp) || !(*ifp)->ndev) {
-		if (ret != -ENODATA && ifp)
+		if (ret != -ENODATA && *ifp)
 			(*ifp)->stats.rx_errors++;
 		brcmu_pkt_buf_free_skb(skb);
 		return -ENODATA;
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 15 lines · Dan Carpenter

Smatch complains about this code:

drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c:335 brcmf_rx_hdrpull()
error: we previously assumed '*ifp' could be null (see line 333)

The problem is that we recently changed these from "ifp" to "*ifp" but there was one that we didn't update.

(quoted diff) +2 -2
-       if (ret || !ifp || !ifp->ndev) {
+       if (ret || !(*ifp) || !(*ifp)->ndev) {
                if (ret != -ENODATA && ifp)
                                       ^^^
-                       ifp->stats.rx_errors++;
+                       (*ifp)->stats.rx_errors++;
Quoted 2 lines

I have updated it to *ifp as well. We always call this function is a non-NULL "ifp" pointer, btw.

Great. Maybe we (Broadcom) should consider running smatch on our patch series or simply rely on your excellent work :-p. Thanks for catching this.

Acked-by Arend van Spriel <[email protected]>
Quoted 17 lines
Fixes c462ebcdfe42 ('brcmfmac: create common function for handling brcmf_proto_hdrpull()')
Signed-off-by Dan Carpenter <[email protected]>
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +1 -1
index 1b476d1..b590499 100644
@@ -331,7 +331,7 @@ static int brcmf_rx_hdrpull(struct brcmf_pub *drvr, struct sk_buff *skb,
 	ret = brcmf_proto_hdrpull(drvr, true, skb, ifp);
 
 	if (ret || !(*ifp) || !(*ifp)->ndev) {
-		if (ret != -ENODATA && ifp)
+		if (ret != -ENODATA && *ifp)
 			(*ifp)->stats.rx_errors++;
 		brcmu_pkt_buf_free_skb(skb);
 		return -ENODATA;
Applied
#2 lore raw
In reply to Dan Carpenter
Quoted 16 lines

Smatch complains about this code:

drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c:335 brcmf_rx_hdrpull()
error: we previously assumed '*ifp' could be null (see line 333)

The problem is that we recently changed these from "ifp" to "*ifp" but there was one that we didn't update.

(quoted diff) +2 -2
-       if (ret || !ifp || !ifp->ndev) {
+       if (ret || !(*ifp) || !(*ifp)->ndev) {
                if (ret != -ENODATA && ifp)
                                       ^^^
-                       ifp->stats.rx_errors++;
+                       (*ifp)->stats.rx_errors++;
Quoted 6 lines

I have updated it to *ifp as well. We always call this function is a non-NULL "ifp" pointer, btw.

Fixes c462ebcdfe42 ('brcmfmac: create common function for handling brcmf_proto_hdrpull()')
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Arend van Spriel <[email protected]>

Thanks, applied to wireless-drivers-next.git.

Kalle Valo