Skip to contentSkip to conversation

staging: rtl8723au: Remove dead code

PATCH In review Supriya Karanth 2 participants 4 messages Mar 12, 2015 – Mar 13, 2015 1 message not in the archive Staging

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: rtl8723au: Remove dead code

This dead code was detected by coccinelle while using --debug option

---
 drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 1 -
 1 file changed, 1 deletion(-)
drivers/staging/rtl8723au/core/rtw_mlme_ext.c +0 -1
index 0e0f73c..db4f235 100644
@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
 			} else {
 				break;
 			}
-			p = p + p[1] + 2;
 		}
 	}
 
Signature · 1 line
2.1.0

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 10 lines · Larry Finger
Quoted 9 lines · Supriya Karanth

This dead code was detected by coccinelle while using --debug option

drivers/staging/rtl8723au/core/rtw_mlme_ext.c +0 -0
index 0e0f73c..db4f235 100644
@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
 			} else {
 				break;
Quoted 8 lines

drivers/staging/rtl8723au/core/rtw_mlme_ext.c OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame) · line 1624

Quoted 5 lines
drivers/staging/rtl8723au/core/rtw_mlme_ext.c +0 -1
 			}
-			p = p + p[1] + 2;
 		}
 	}
 

Like most of this driver, the code is ugly, but I think this is a coccinelle bug, not dead code.

I checked it myself and thought coccinelle was right

Quoted 5 lines

The statement that you are removing is at the end of a for loop. On the next pass through the loop, and there will be a next pass, the first statement is "left = end - p;"; therefore, the incrementation of p is not useless code.

I was under the impression that the line "p = p + p[1] + 2;" will never execute.

The for loop is in this form:

for (;;) {
	if (p) {
		if (...) {
		...

		}
		break;
	} else {
		break;
	}
	p = p ....
}

There is a "break" in both the if and else condition which will break out of the for loop. I think the line "p = p + p[1] + 2;" is important but, I think it never gets executed. Perhaps I am wrong and I am missing something.

Quoted 2 lines

Have you tested your change? I would guess not. I think it would lead to an infinite loop.

I am sorry but no I haven't tested it as I do not have the Hardware. I have tested the code format though and the line doesn't get executed.

Quoted 3 lines

The recent spate of untested patches generated by uncritical usage of various tools is quite tiresome!!

I understand. I am very sorry to have wasted your time.

Quoted 6 lines

NACK

Larry

#3 lore raw
Quoted 50 lines · Supriya Karanth
Quoted 10 lines · Larry Finger
Quoted 9 lines · Supriya Karanth

This dead code was detected by coccinelle while using --debug option

drivers/staging/rtl8723au/core/rtw_mlme_ext.c +0 -0
index 0e0f73c..db4f235 100644
@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
 			} else {
 				break;
Quoted 8 lines

drivers/staging/rtl8723au/core/rtw_mlme_ext.c OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame) · line 1624

Quoted 5 lines
drivers/staging/rtl8723au/core/rtw_mlme_ext.c +0 -1
 			}
-			p = p + p[1] + 2;
 		}
 	}
 

Like most of this driver, the code is ugly, but I think this is a coccinelle bug, not dead code.

I checked it myself and thought coccinelle was right

Quoted 5 lines

The statement that you are removing is at the end of a for loop. On the next pass through the loop, and there will be a next pass, the first statement is "left = end - p;"; therefore, the incrementation of p is not useless code.

I was under the impression that the line "p = p + p[1] + 2;" will never execute.

The for loop is in this form:

for (;;) {
	if (p) {
		if (...) {
		...

		}
		break;
	} else {
		break;
	}
	p = p ....
}

There is a "break" in both the if and else condition which will break out of the for loop. I think the line "p = p + p[1] + 2;" is important but, I think it never gets executed. Perhaps I am wrong and I am missing something.

Quoted 2 lines

Have you tested your change? I would guess not. I think it would lead to an infinite loop.

Quoted 9 lines

I am sorry but no I haven't tested it as I do not have the Hardware. I have tested the code format though and the line doesn't get executed.

Quoted 3 lines

The recent spate of untested patches generated by uncritical usage of various tools is quite tiresome!!

I understand. I am very sorry to have wasted your time.

Quoted 2 lines

NACK

I agree with Supriya. I can't see how the code could be executed. Maybe her fix is not correct, but something is wrong with the code.

julia

Quoted 12 lines
Quoted 4 lines

Larry

Signature · 5 lines
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/20150313035724.GA6885%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
#4 lore raw
Quoted 60 lines · Julia Lawall
Quoted 56 lines · Supriya Karanth
Quoted 10 lines · Larry Finger
Quoted 9 lines · Supriya Karanth

This dead code was detected by coccinelle while using --debug option

drivers/staging/rtl8723au/core/rtw_mlme_ext.c +0 -0
index 0e0f73c..db4f235 100644
@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
 			} else {
 				break;
Quoted 8 lines

drivers/staging/rtl8723au/core/rtw_mlme_ext.c OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame) · line 1624

Quoted 5 lines
drivers/staging/rtl8723au/core/rtw_mlme_ext.c +0 -1
 			}
-			p = p + p[1] + 2;
 		}
 	}
 

Like most of this driver, the code is ugly, but I think this is a coccinelle bug, not dead code.

I checked it myself and thought coccinelle was right

Quoted 5 lines

The statement that you are removing is at the end of a for loop. On the next pass through the loop, and there will be a next pass, the first statement is "left = end - p;"; therefore, the incrementation of p is not useless code.

I was under the impression that the line "p = p + p[1] + 2;" will never execute.

The for loop is in this form:

for (;;) {
	if (p) {
		if (...) {
		...

		}
		break;
	} else {
		break;
	}
	p = p ....
}

There is a "break" in both the if and else condition which will break out of the for loop. I think the line "p = p + p[1] + 2;" is important but, I think it never gets executed. Perhaps I am wrong and I am missing something.

Quoted 2 lines

Have you tested your change? I would guess not. I think it would lead to an infinite loop.

I am sorry but no I haven't tested it as I do not have the Hardware. I have tested the code format though and the line doesn't get executed.

Quoted 3 lines

The recent spate of untested patches generated by uncritical usage of various tools is quite tiresome!!

Quoted 6 lines
Quoted 3 lines

I understand. I am very sorry to have wasted your time.

Quoted 2 lines

NACK

I agree with Supriya. I can't see how the code could be executed. Maybe her fix is not correct, but something is wrong with the code.

The problem was introduced in the patch 7c3a8f2a5e. Prior to that patch, the break that is now below if (pstat->qos_info & 0xf) was inside the "then" branch of that if. The commit message of that patch doesn't describe the changes in detail, so it is not clear why this change was made. However, the patch is from Jes, so maybe he remembers.

julia