[PATCH] staging: rtl8723au: Remove dead code

[PATCH] staging: rtl8723au: Remove dead code

[Supriya Karanth]

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

Signed-off-by: Supriya Karanth <iskaranth@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
index 0e0f73c..db4f235 100644
--- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
 			} else {
 				break;
 			}
-			p = p + p[1] + 2;
 		}
 	}
 
-- 
2.1.0

Re: [PATCH] staging: rtl8723au: Remove dead code

[Supriya Karanth]

On Thu, Mar 12, 2015 at 11:53:08AM -0500, Larry Finger wrote:
> On 03/12/2015 06:10 AM, Supriya Karanth wrote:
> >This dead code was detected by coccinelle while using --debug
> >option
> >diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> >index 0e0f73c..db4f235 100644
> >--- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> >+++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> >@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
> >  			} else {
> >  				break;
> >  			}
> >-			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
> 
> 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.

> 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.
> 
> 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.
> 
> NACK
> 
> Larry
> 
> 

Re: [Outreachy kernel] Re: [PATCH] staging: rtl8723au: Remove dead code

[Julia Lawall]

On Fri, 13 Mar 2015, Supriya Karanth wrote:

> On Thu, Mar 12, 2015 at 11:53:08AM -0500, Larry Finger wrote:
> > On 03/12/2015 06:10 AM, Supriya Karanth wrote:
> > >This dead code was detected by coccinelle while using --debug
> > >option
> > >diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > >index 0e0f73c..db4f235 100644
> > >--- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > >+++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > >@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
> > >  			} else {
> > >  				break;
> > >  			}
> > >-			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
> > 
> > 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.
> 
> > 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.
> > 
> > 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.
> > 
> > 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



> > 
> > Larry
> > 
> > 
> 
> -- 
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
> 

Re: [Outreachy kernel] Re: [PATCH] staging: rtl8723au: Remove dead code

[Julia Lawall]

On Fri, 13 Mar 2015, Julia Lawall wrote:

> 
> 
> On Fri, 13 Mar 2015, Supriya Karanth wrote:
> 
> > On Thu, Mar 12, 2015 at 11:53:08AM -0500, Larry Finger wrote:
> > > On 03/12/2015 06:10 AM, Supriya Karanth wrote:
> > > >This dead code was detected by coccinelle while using --debug
> > > >option
> > > >diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > > >index 0e0f73c..db4f235 100644
> > > >--- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > > >+++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > > >@@ -1619,7 +1619,6 @@ OnAssocReq23a(struct rtw_adapter *padapter, struct recv_frame *precv_frame)
> > > >  			} else {
> > > >  				break;
> > > >  			}
> > > >-			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
> > > 
> > > 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.
> > 
> > > 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.
> > > 
> > > 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.
> > > 
> > > 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