Staging: rtl8723au: rtw_xmit: Remove extra NULL tests

PATCH Superseded Shivani Bhardwaj 2 participants 10 messages Oct 14, 2015 Staging

Share & tools

Shivani Bhardwaj

author

2015-10-14 00:51 UTC #0 lore raw

PATCH 1/1 Staging: rtl8723au: rtw_xmit: Remove extra NULL tests

Variables already tested for NULL value are checked again so, the extra NULL tests should be removed.
Semantic patch used:

@@ expression E; @@
if(E==NULL) {... return...;}
- if(E)

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 -9

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
 	if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
 		/* encode mic code */
-		if (stainfo) {
 			u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
 					 0x0, 0x0, 0x0, 0x0,
 					 0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 					 *(payload + curfragnum + 6),
 					 *(payload + curfragnum + 7));
 			}
-		} else {
-			RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-				 "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-		}
 	}
 
 	return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
 		/* TODO: fill HT Control Field */
 
 		/* Update Seq Num will be handled by f/w */
-		if (psta) {
 			psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
 			psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
 			pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
 					pattrib->ampdu_en = true;/* AGG EN */
 				}
 			}
-		}
 	}
 exit:
 	return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
 		/* adding icv, if necessary... */
 		if (pattrib->iv_len) {
-			if (psta) {
 				switch (pattrib->encrypt) {
 				case WLAN_CIPHER_SUITE_WEP40:
 				case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 						       psta->dot11txpn, 0);
 					break;
 				}
-			}
 
 			memcpy(pframe, pattrib->iv, pattrib->iv_len);

Signature · 1 line

2.1.0

Julia Lawall

2015-10-14 05:48 UTC #1 lore raw

Quoted 7 lines · Shivani Bhardwaj

Variables already tested for NULL value are checked again so, the extra NULL tests should be removed.
Semantic patch used:
@@ expression E; @@
if(E==NULL) {... return...;}
- if(E)

This is not a valid semantic patch. The if needs at least a then branch.

julia

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 -9

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
 	if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
 		/* encode mic code */
-		if (stainfo) {
 			u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
 					 0x0, 0x0, 0x0, 0x0,
 					 0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 					 *(payload + curfragnum + 6),
 					 *(payload + curfragnum + 7));
 			}
-		} else {
-			RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-				 "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-		}
 	}
 
 	return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
 		/* TODO: fill HT Control Field */
 
 		/* Update Seq Num will be handled by f/w */
-		if (psta) {
 			psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
 			psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
 			pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
 					pattrib->ampdu_en = true;/* AGG EN */
 				}
 			}
-		}
 	}
 exit:
 	return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
 		/* adding icv, if necessary... */
 		if (pattrib->iv_len) {
-			if (psta) {
 				switch (pattrib->encrypt) {
 				case WLAN_CIPHER_SUITE_WEP40:
 				case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 						       psta->dot11txpn, 0);
 					break;
 				}
-			}
 
 			memcpy(pframe, pattrib->iv, pattrib->iv_len);

Signature · 8 lines

2.1.0

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151014005112.GA26569%40ubuntu.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151014005112.GA26569%40ubuntu.
For more options, visit https://groups.google.com/d/optout.

Shivani Bhardwaj

author

2015-10-14 09:09 UTC #3 lore raw

Quoted 11 lines · Shivani Bhardwaj

Quoted 10 lines · Julia Lawall
Quoted 7 lines · Shivani Bhardwaj
Variables already tested for NULL value are checked again so, the extra NULL tests should be removed.
Semantic patch used:
@@ expression E; @@
if(E==NULL) {... return...;}
- if(E)

Quoted 7 lines

Quoted 5 lines
This is not a valid semantic patch. The if needs at least a then branch.
julia
@@ expression E;

statement S; @@

Quoted 81 lines

if(E==NULL) {... return ...;}
else S

(quoted diff) +1 -1

-if(E==NULL){
+{
...

}
-else S

Can you check this one, Julia?

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 ~~-10~~

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
      if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
              /* encode mic code */
-             if (stainfo) {
                      u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
                                       *(payload + curfragnum + 6),
                                       *(payload + curfragnum + 7));
                      }
-             } else {
-                     RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-                              "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-             }
      }
 
      return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
              /* TODO: fill HT Control Field */
 
              /* Update Seq Num will be handled by f/w */
-             if (psta) {
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
                      pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
                                      pattrib->ampdu_en = true;/* AGG EN */
                              }
                      }
-             }
      }
 exit:
      return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
              /* adding icv, if necessary... */
              if (pattrib->iv_len) {
-                     if (psta) {
                              switch (pattrib->encrypt) {
                              case WLAN_CIPHER_SUITE_WEP40:
                              case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
                                                     psta->dot11txpn, 0);
                                      break;
                              }
-                     }
 
                      memcpy(pframe, pattrib->iv, pattrib->iv_len);
 
--

2.1.0

Julia Lawall

2015-10-14 09:14 UTC #4 lore raw

Quoted 19 lines · Shivani Bhardwaj

Quoted 11 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 7 lines · Shivani Bhardwaj
Variables already tested for NULL value are checked again so, the extra NULL tests should be removed.
Semantic patch used:
@@ expression E; @@
if(E==NULL) {... return...;}
- if(E)
Quoted 7 lines
Quoted 5 lines
This is not a valid semantic patch. The if needs at least a then branch.
julia
@@ expression E;

Quoted 8 lines

statement S; @@
Quoted 7 lines
if(E==NULL) {... return ...;}
else S
(quoted diff) +1 -1
-if(E==NULL){
+{
...
}
-else S

It is strange to remove the { and add it back. Your semantic patch can have more newlines than the actual C code, so you don't have to put the { on the same line as the if.

julia

Quoted 75 lines

Quoted 74 lines

Can you check this one, Julia?

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 ~~-10~~

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
      if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
              /* encode mic code */
-             if (stainfo) {
                      u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
                                       *(payload + curfragnum + 6),
                                       *(payload + curfragnum + 7));
                      }
-             } else {
-                     RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-                              "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-             }
      }
 
      return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
              /* TODO: fill HT Control Field */
 
              /* Update Seq Num will be handled by f/w */
-             if (psta) {
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
                      pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
                                      pattrib->ampdu_en = true;/* AGG EN */
                              }
                      }
-             }
      }
 exit:
      return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
              /* adding icv, if necessary... */
              if (pattrib->iv_len) {
-                     if (psta) {
                              switch (pattrib->encrypt) {
                              case WLAN_CIPHER_SUITE_WEP40:
                              case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
                                                     psta->dot11txpn, 0);
                                      break;
                              }
-                     }
 
                      memcpy(pframe, pattrib->iv, pattrib->iv_len);
 
--

2.1.0

Shivani Bhardwaj

author

2015-10-14 09:17 UTC #5 lore raw

Quoted 36 lines · Julia Lawall

Quoted 19 lines · Shivani Bhardwaj
Quoted 11 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 7 lines · Shivani Bhardwaj
Variables already tested for NULL value are checked again so, the extra NULL tests should be removed.
Semantic patch used:
@@ expression E; @@
if(E==NULL) {... return...;}
- if(E)
Quoted 7 lines
Quoted 5 lines
This is not a valid semantic patch. The if needs at least a then branch.
julia
@@ expression E;
Quoted 8 lines
statement S; @@
Quoted 7 lines
if(E==NULL) {... return ...;}
else S
(quoted diff) +1 -1
-if(E==NULL){
+{
...
}
-else S
It is strange to remove the { and add it back. Your semantic patch can have more newlines than the actual C code, so you don't have to put the { on the same line as the if.

Quoted 2 lines

julia

@@ expression E;
statement S; @@
if(E==NULL) {... return ...;}
else S
- if(E==NULL)
{
...
}
- else S

How about this?

Quoted 75 lines

Quoted 74 lines

Can you check this one, Julia?

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 ~~-10~~

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
      if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
              /* encode mic code */
-             if (stainfo) {
                      u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
                                       *(payload + curfragnum + 6),
                                       *(payload + curfragnum + 7));
                      }
-             } else {
-                     RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-                              "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-             }
      }
 
      return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
              /* TODO: fill HT Control Field */
 
              /* Update Seq Num will be handled by f/w */
-             if (psta) {
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
                      pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
                                      pattrib->ampdu_en = true;/* AGG EN */
                              }
                      }
-             }
      }
 exit:
      return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
              /* adding icv, if necessary... */
              if (pattrib->iv_len) {
-                     if (psta) {
                              switch (pattrib->encrypt) {
                              case WLAN_CIPHER_SUITE_WEP40:
                              case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
                                                     psta->dot11txpn, 0);
                                      break;
                              }
-                     }
 
                      memcpy(pframe, pattrib->iv, pattrib->iv_len);
 
--

2.1.0

Julia Lawall

2015-10-14 09:51 UTC #6 lore raw

Quoted 11 lines

@@ expression E;
statement S; @@
if(E==NULL) {... return ...;}
else S
- if(E==NULL)
{
...
}
- else S
How about this?

That's better. But actually, you don't care about whether there are braces, so you could just put S1.

Actually, there is also no need to require that the else of the second if is the same as the else of the first one. So you should have a different statement metavariable in each case.

julia

Quoted 77 lines

Quoted 76 lines

Quoted 75 lines

Can you check this one, Julia?

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 ~~-10~~

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
      if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
              /* encode mic code */
-             if (stainfo) {
                      u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
                                       *(payload + curfragnum + 6),
                                       *(payload + curfragnum + 7));
                      }
-             } else {
-                     RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-                              "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-             }
      }
 
      return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
              /* TODO: fill HT Control Field */
 
              /* Update Seq Num will be handled by f/w */
-             if (psta) {
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
                      pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
                                      pattrib->ampdu_en = true;/* AGG EN */
                              }
                      }
-             }
      }
 exit:
      return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
              /* adding icv, if necessary... */
              if (pattrib->iv_len) {
-                     if (psta) {
                              switch (pattrib->encrypt) {
                              case WLAN_CIPHER_SUITE_WEP40:
                              case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
                                                     psta->dot11txpn, 0);
                                      break;
                              }
-                     }
 
                      memcpy(pframe, pattrib->iv, pattrib->iv_len);
 
--

2.1.0

Shivani Bhardwaj

author

2015-10-14 09:54 UTC #7 lore raw

Quoted 12 lines · Julia Lawall

Quoted 11 lines
@@ expression E;
statement S; @@
if(E==NULL) {... return ...;}
else S
- if(E==NULL)
{
...
}
- else S
How about this?

Quoted 9 lines

That's better. But actually, you don't care about whether there are braces, so you could just put S1.
Actually, there is also no need to require that the else of the second if is the same as the else of the first one. So you should have a different statement metavariable in each case.
julia

@@ expression E;
statement S1, S2; @@
if(E==NULL) {... return ...;}
else S1
- if(E==NULL)
- else S2

This should be fine, I think?

Quoted 77 lines

Quoted 76 lines

Quoted 75 lines

Can you check this one, Julia?

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 ~~-10~~

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
      if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
              /* encode mic code */
-             if (stainfo) {
                      u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
                                       *(payload + curfragnum + 6),
                                       *(payload + curfragnum + 7));
                      }
-             } else {
-                     RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-                              "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-             }
      }
 
      return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
              /* TODO: fill HT Control Field */
 
              /* Update Seq Num will be handled by f/w */
-             if (psta) {
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
                      pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
                                      pattrib->ampdu_en = true;/* AGG EN */
                              }
                      }
-             }
      }
 exit:
      return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
              /* adding icv, if necessary... */
              if (pattrib->iv_len) {
-                     if (psta) {
                              switch (pattrib->encrypt) {
                              case WLAN_CIPHER_SUITE_WEP40:
                              case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
                                                     psta->dot11txpn, 0);
                                      break;
                              }
-                     }
 
                      memcpy(pframe, pattrib->iv, pattrib->iv_len);
 
--

2.1.0

Julia Lawall

2015-10-14 10:00 UTC #8 lore raw

Quoted 22 lines · Shivani Bhardwaj

Quoted 12 lines · Julia Lawall
Quoted 11 lines
@@ expression E;
statement S; @@
if(E==NULL) {... return ...;}
else S
- if(E==NULL)
{
...
}
- else S
How about this?
Quoted 9 lines
That's better. But actually, you don't care about whether there are braces, so you could just put S1.
Actually, there is also no need to require that the else of the second if is the same as the else of the first one. So you should have a different statement metavariable in each case.
julia

Quoted 8 lines

@@ expression E;
statement S1, S2; @@
if(E==NULL) {... return ...;}
else S1
- if(E==NULL)
- else S2
This should be fine, I think?

No, an if has two branches, then an else. You can omit the else, but here you have omitted the then, so it doesn't work.

Actually, you can check your semantic patch by putting it in a file file.cocci, and then running spatch --parse-cocci file.cocci

julia

Quoted 79 lines

Quoted 78 lines

Quoted 77 lines

Quoted 76 lines

Can you check this one, Julia?

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 ~~-10~~

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
      if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
              /* encode mic code */
-             if (stainfo) {
                      u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
                                       *(payload + curfragnum + 6),
                                       *(payload + curfragnum + 7));
                      }
-             } else {
-                     RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-                              "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-             }
      }
 
      return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
              /* TODO: fill HT Control Field */
 
              /* Update Seq Num will be handled by f/w */
-             if (psta) {
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
                      pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
                                      pattrib->ampdu_en = true;/* AGG EN */
                              }
                      }
-             }
      }
 exit:
      return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
              /* adding icv, if necessary... */
              if (pattrib->iv_len) {
-                     if (psta) {
                              switch (pattrib->encrypt) {
                              case WLAN_CIPHER_SUITE_WEP40:
                              case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
                                                     psta->dot11txpn, 0);
                                      break;
                              }
-                     }
 
                      memcpy(pframe, pattrib->iv, pattrib->iv_len);
 
--

2.1.0

Shivani Bhardwaj

author

2015-10-14 10:02 UTC #9 lore raw

Quoted 35 lines · Julia Lawall

Quoted 22 lines · Shivani Bhardwaj
Quoted 12 lines · Julia Lawall
Quoted 11 lines
@@ expression E;
statement S; @@
if(E==NULL) {... return ...;}
else S
- if(E==NULL)
{
...
}
- else S
How about this?
Quoted 9 lines
That's better. But actually, you don't care about whether there are braces, so you could just put S1.
Actually, there is also no need to require that the else of the second if is the same as the else of the first one. So you should have a different statement metavariable in each case.
julia
Quoted 8 lines
@@ expression E;
statement S1, S2; @@
if(E==NULL) {... return ...;}
else S1
- if(E==NULL)
- else S2
This should be fine, I think?

Quoted 8 lines

No, an if has two branches, then an else. You can omit the else, but here you have omitted the then, so it doesn't work.
Actually, you can check your semantic patch by putting it in a file file.cocci, and then running spatch --parse-cocci file.cocci
julia

Right! I omitted the content inside if, which should not happen. I get it now. Thanks, Julia! :)

Quoted 79 lines

Quoted 78 lines

Quoted 77 lines

Quoted 76 lines

Can you check this one, Julia?

Quoted 71 lines

Signed-off-by Shivani Bhardwaj <[email protected]>

---
 drivers/staging/rtl8723au/core/rtw_xmit.c | 9 ---------
 1 file changed, 9 deletions(-)

drivers/staging/rtl8723au/core/rtw_xmit.c +0 ~~-10~~

index a4b6bb6..0ea521b 100644
@@ -667,7 +667,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
 
      if (pattrib->encrypt == WLAN_CIPHER_SUITE_TKIP) {
              /* encode mic code */
-             if (stainfo) {
                      u8 null_key[16]={0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,
                                       0x0, 0x0, 0x0, 0x0,

@@ -792,10 +791,6 @@ static int xmitframe_addmic(struct rtw_adapter *padapter,
                                       *(payload + curfragnum + 6),
                                       *(payload + curfragnum + 7));
                      }
-             } else {
-                     RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
-                              "xmitframe_addmic: rtw_get_stainfo23a ==NULL!!!\n");
-             }
      }
 
      return _SUCCESS;

@@ -934,7 +929,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
              /* TODO: fill HT Control Field */
 
              /* Update Seq Num will be handled by f/w */
-             if (psta) {
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority]++;
                      psta->sta_xmitpriv.txseq_tid[pattrib->priority] &= 0xFFF;
                      pattrib->seqnum = psta->sta_xmitpriv.txseq_tid[pattrib->priority];

@@ -970,7 +964,6 @@ static int rtw_make_wlanhdr(struct rtw_adapter *padapter, u8 *hdr,
                                      pattrib->ampdu_en = true;/* AGG EN */
                              }
                      }
-             }
      }
 exit:
      return res;

@@ -1132,7 +1125,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
 
              /* adding icv, if necessary... */
              if (pattrib->iv_len) {
-                     if (psta) {
                              switch (pattrib->encrypt) {
                              case WLAN_CIPHER_SUITE_WEP40:
                              case WLAN_CIPHER_SUITE_WEP104:

@@ -1158,7 +1150,6 @@ int rtw_xmitframe_coalesce23a(struct rtw_adapter *padapter, struct sk_buff *skb,
                                                     psta->dot11txpn, 0);
                                      break;
                              }
-                     }
 
                      memcpy(pframe, pattrib->iv, pattrib->iv_len);
 
--

2.1.0