[PATCH] staging: rtl8723au: Fixes unnecessary return warning

[PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Bhaktipriya Shridhar]

This patch fixes checkpatch.pl warning in rtw_mlme_ext.c file.
WARNING: void function return statements are not generally useful

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
index d28f29a..e8a16b9 100644
--- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
@@ -2657,7 +2657,6 @@ static void issue_probersp(struct rtw_adapter *padapter, unsigned char *da)

 	dump_mgntframe23a(padapter, pmgntframe);

-	return;
 }

 static int _issue_probereq(struct rtw_adapter *padapter,
@@ -2958,7 +2957,6 @@ static void issue_auth(struct rtw_adapter *padapter, struct sta_info *psta,
 	DBG_8723A("%s\n", __func__);
 	dump_mgntframe23a(padapter, pmgntframe);

-	return;
 }

 #ifdef CONFIG_8723AU_AP_MODE
@@ -3339,7 +3337,6 @@ exit:
 	} else
 		kfree(pmlmepriv->assoc_req);

-	return;
 }

 /* when wait_ack is true, this function should be called at process context */
@@ -4103,7 +4100,6 @@ static void rtw_site_survey(struct rtw_adapter *padapter)
 		pmlmeext->sitesurvey_res.state = SCAN_DISABLE;
 	}

-	return;
 }

 /* collect bss info from Beacon and Probe request/response frames. */
@@ -4760,7 +4756,6 @@ void report_survey_event23a(struct rtw_adapter *padapter,

 	pmlmeext->sitesurvey_res.bss_cnt++;

-	return;
 }

 void report_surveydone_event23a(struct rtw_adapter *padapter)
@@ -4803,7 +4798,6 @@ void report_surveydone_event23a(struct rtw_adapter *padapter)

 	rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);

-	return;
 }

 void report_join_res23a(struct rtw_adapter *padapter, int res)
@@ -4851,7 +4845,6 @@ void report_join_res23a(struct rtw_adapter *padapter, int res)

 	rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);

-	return;
 }

 void report_del_sta_event23a(struct rtw_adapter *padapter,
@@ -4907,7 +4900,6 @@ void report_del_sta_event23a(struct rtw_adapter *padapter,

 	rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);

-	return;
 }

 void report_add_sta_event23a(struct rtw_adapter *padapter,
@@ -4952,7 +4944,6 @@ void report_add_sta_event23a(struct rtw_adapter *padapter,

 	rtw_enqueue_cmd23a(pcmdpriv, pcmd_obj);

-	return;
 }

 /****************************************************************************
@@ -5395,7 +5386,6 @@ static void link_timer_hdl(unsigned long data)
 		set_link_timer(pmlmeext, REASSOC_TO);
 	}

-	return;
 }

 static void addba_timer_hdl(unsigned long data)
--
2.1.4

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Jes Sorensen]

Bhaktipriya Shridhar <bhaktipriya96@gmail.com> writes:
> This patch fixes checkpatch.pl warning in rtw_mlme_ext.c file.
> WARNING: void function return statements are not generally useful
>
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> index d28f29a..e8a16b9 100644
> --- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> @@ -2657,7 +2657,6 @@ static void issue_probersp(struct rtw_adapter *padapter, unsigned char *da)
>
>  	dump_mgntframe23a(padapter, pmgntframe);
>
> -	return;
>  }

If you insist on pushing this rather unncessary change, please do it
properly, and remove the blank line before the return statement as well.

Jes

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Julian Calaby]

Hi Bhaktipriya,

On Sat, Jan 30, 2016 at 5:00 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> Bhaktipriya Shridhar <bhaktipriya96@gmail.com> writes:
>> This patch fixes checkpatch.pl warning in rtw_mlme_ext.c file.
>> WARNING: void function return statements are not generally useful
>>
>> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
>> ---
>>  drivers/staging/rtl8723au/core/rtw_mlme_ext.c | 10 ----------
>>  1 file changed, 10 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
>> index d28f29a..e8a16b9 100644
>> --- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
>> +++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
>> @@ -2657,7 +2657,6 @@ static void issue_probersp(struct rtw_adapter *padapter, unsigned char *da)
>>
>>       dump_mgntframe23a(padapter, pmgntframe);
>>
>> -     return;
>>  }
>
> If you insist on pushing this rather unncessary change, please do it
> properly, and remove the blank line before the return statement as well.

As Jes said, you need to remove the blank lines before the returns
too. checkpatch should have picked this up, you did run the patch
through checkpatch before you sent it, right?

Jes,

I know you have strong feelings on coding style, but there are a lot
of people out there who see deviations from the standard as bugs to be
fixed, so stuff like this isn't going to stop until it matches the
coding style document's spec.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Joe Perches]

On Sat, 2016-01-30 at 10:17 +1100, Julian Calaby wrote:
> On Sat, Jan 30, 2016 at 5:00 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> > Bhaktipriya Shridhar <bhaktipriya96@gmail.com> writes:
> > > This patch fixes checkpatch.pl warning in rtw_mlme_ext.c file.
> > > WARNING: void function return statements are not generally useful
[]
> > > diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
[]
> > > @@ -2657,7 +2657,6 @@ static void issue_probersp(struct rtw_adapter *padapter, unsigned char *da)
> > > 
> > >       dump_mgntframe23a(padapter, pmgntframe);
> > > 
> > > -     return;
> > >  }
> > 
> > If you insist on pushing this rather unncessary change, please do it
> > properly, and remove the blank line before the return statement as well.
> 
> As Jes said, you need to remove the blank lines before the returns
> too. checkpatch should have picked this up, you did run the patch
> through checkpatch before you sent it, right?

checkpatch doesn't pick this up.

If you'd like to make it do so, you're welcome to try
but it's likely a bit more complicated than it appears.

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Julian Calaby]

Hi Joe,

On Sat, Jan 30, 2016 at 12:28 PM, Joe Perches <joe@perches.com> wrote:
> On Sat, 2016-01-30 at 10:17 +1100, Julian Calaby wrote:
>> On Sat, Jan 30, 2016 at 5:00 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>> > Bhaktipriya Shridhar <bhaktipriya96@gmail.com> writes:
>> > > This patch fixes checkpatch.pl warning in rtw_mlme_ext.c file.
>> > > WARNING: void function return statements are not generally useful
> []
>> > > diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> []
>> > > @@ -2657,7 +2657,6 @@ static void issue_probersp(struct rtw_adapter *padapter, unsigned char *da)
>> > >
>> > >       dump_mgntframe23a(padapter, pmgntframe);
>> > >
>> > > -     return;
>> > >  }
>> >
>> > If you insist on pushing this rather unncessary change, please do it
>> > properly, and remove the blank line before the return statement as well.
>>
>> As Jes said, you need to remove the blank lines before the returns
>> too. checkpatch should have picked this up, you did run the patch
>> through checkpatch before you sent it, right?
>
> checkpatch doesn't pick this up.
>
> If you'd like to make it do so, you're welcome to try
> but it's likely a bit more complicated than it appears.

I meant the extra blank lines, not the useless return statements.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Joe Perches]

On Sat, 2016-01-30 at 14:09 +1100, Julian Calaby wrote:
> Hi Joe,

Hello Julian.

> On Sat, Jan 30, 2016 at 12:28 PM, Joe Perches <joe@perches.com>
> wrote:
> > On Sat, 2016-01-30 at 10:17 +1100, Julian Calaby wrote:
> > > On Sat, Jan 30, 2016 at 5:00 AM, Jes Sorensen <Jes.Sorensen@redha
> > > t.com> wrote:
> > > > Bhaktipriya Shridhar <bhaktipriya96@gmail.com> writes:
> > > > > This patch fixes checkpatch.pl warning in rtw_mlme_ext.c
> > > > > file.
> > > > > WARNING: void function return statements are not generally
> > > > > useful
> > []
> > > > > diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > > > > b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
> > []
> > > > > @@ -2657,7 +2657,6 @@ static void issue_probersp(struct
> > > > > rtw_adapter *padapter, unsigned char *da)
> > > > > 
> > > > >       dump_mgntframe23a(padapter, pmgntframe);
> > > > > 
> > > > > -     return;
> > > > >  }
> > > > 
> > > > If you insist on pushing this rather unncessary change, please
> > > > do it
> > > > properly, and remove the blank line before the return statement
> > > > as well.
> > > 
> > > As Jes said, you need to remove the blank lines before the
> > > returns
> > > too. checkpatch should have picked this up, you did run the patch
> > > through checkpatch before you sent it, right?
> > 
> > checkpatch doesn't pick this up.
> > 
> > If you'd like to make it do so, you're welcome to try
> > but it's likely a bit more complicated than it appears.
> 
> I meant the extra blank lines, not the useless return statements.

I understood what you meant.

It's relatively difficult to determine that a line removal
patch causes a blank line to appear before a closing brace.

You're welcome to extend checkpatch to find these things,
but there are likely many additional patch types that need
to be considered.  Remember patches can add, modify and
delete lines.

cheers, Joe

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Bhakti Priya]

Hi,

Thank you for your reply. I've just sent version 2 of the patch with
the blank lines removed.
I will be happy to extend checkpatch.pl. As suggested by you, I am
trying to detect such blank lines in a line removal patch by checking
if the line above the deleted line was a blank line and the line
following the deleted line had a closing brace.
Can you please guide me and let me know if I am headed in the right direction.

Thanks,
Bhaktipriya


On Sat, Jan 30, 2016 at 8:48 AM, Joe Perches <joe@perches.com> wrote:
> On Sat, 2016-01-30 at 14:09 +1100, Julian Calaby wrote:
>> Hi Joe,
>
> Hello Julian.
>
>> On Sat, Jan 30, 2016 at 12:28 PM, Joe Perches <joe@perches.com>
>> wrote:
>> > On Sat, 2016-01-30 at 10:17 +1100, Julian Calaby wrote:
>> > > On Sat, Jan 30, 2016 at 5:00 AM, Jes Sorensen <Jes.Sorensen@redha
>> > > t.com> wrote:
>> > > > Bhaktipriya Shridhar <bhaktipriya96@gmail.com> writes:
>> > > > > This patch fixes checkpatch.pl warning in rtw_mlme_ext.c
>> > > > > file.
>> > > > > WARNING: void function return statements are not generally
>> > > > > useful
>> > []
>> > > > > diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
>> > > > > b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c
>> > []
>> > > > > @@ -2657,7 +2657,6 @@ static void issue_probersp(struct
>> > > > > rtw_adapter *padapter, unsigned char *da)
>> > > > >
>> > > > >       dump_mgntframe23a(padapter, pmgntframe);
>> > > > >
>> > > > > -     return;
>> > > > >  }
>> > > >
>> > > > If you insist on pushing this rather unncessary change, please
>> > > > do it
>> > > > properly, and remove the blank line before the return statement
>> > > > as well.
>> > >
>> > > As Jes said, you need to remove the blank lines before the
>> > > returns
>> > > too. checkpatch should have picked this up, you did run the patch
>> > > through checkpatch before you sent it, right?
>> >
>> > checkpatch doesn't pick this up.
>> >
>> > If you'd like to make it do so, you're welcome to try
>> > but it's likely a bit more complicated than it appears.
>>
>> I meant the extra blank lines, not the useless return statements.
>
> I understood what you meant.
>
> It's relatively difficult to determine that a line removal
> patch causes a blank line to appear before a closing brace.
>
> You're welcome to extend checkpatch to find these things,
> but there are likely many additional patch types that need
> to be considered.  Remember patches can add, modify and
> delete lines.
>
> cheers, Joe

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Joe Perches]

On Sat, 2016-01-30 at 12:23 +0530, Bhakti Priya wrote:
> I will be happy to extend checkpatch.pl. As suggested by you, I am
> trying to detect such blank lines in a line removal patch by checking
> if the line above the deleted line was a blank line and the line
> following the deleted line had a closing brace.
> Can you please guide me and let me know if I am headed in the right
> direction.

You have to track all the + and - lines that precede the
deleted line.

Good luck.

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Julian Calaby]

Hi Bhakti,

On Sat, Jan 30, 2016 at 5:53 PM, Bhakti Priya <bhaktipriya96@gmail.com> wrote:
> Hi,
>
> Thank you for your reply. I've just sent version 2 of the patch with
> the blank lines removed.
> I will be happy to extend checkpatch.pl. As suggested by you, I am
> trying to detect such blank lines in a line removal patch by checking
> if the line above the deleted line was a blank line and the line
> following the deleted line had a closing brace.
> Can you please guide me and let me know if I am headed in the right direction.

As I understand it, the algorithm needs to work like this:
1. For each patch hunk:
2. Filter out all lines that match /^-/
3. Remove the first character (" " or "+")
4. Normalise EOL characters: s/\r\n?/\n/
5. Over the entire hunk, find any case that matches
/({|\n)\s*\n\s*(\n|})/ where \s matches all space characters except
\n.
6. Report the middle line the preceding regular expression matches to the user.

I'm confident I can write it as a shell script, but I don't know
enough Perl to add that test to checkpatch.pl

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Joe Perches]

On Sat, 2016-01-30 at 23:02 +1100, Julian Calaby wrote:
> Hi Bhakti,
> 
> On Sat, Jan 30, 2016 at 5:53 PM, Bhakti Priya  wrote:
> > Hi,
> > 
> > Thank you for your reply. I've just sent version 2 of the patch with
> > the blank lines removed.
> > I will be happy to extend checkpatch.pl. As suggested by you, I am
> > trying to detect such blank lines in a line removal patch by checking
> > if the line above the deleted line was a blank line and the line
> > following the deleted line had a closing brace.
> > Can you please guide me and let me know if I am headed in the right direction.
> 
> As I understand it, the algorithm needs to work like this:
> 1. For each patch hunk:
> 2. Filter out all lines that match /^-/
> 3. Remove the first character (" " or "+")
> 4. Normalise EOL characters: s/\r\n?/\n/
> 5. Over the entire hunk, find any case that matches
> /({|\n)\s*\n\s*(\n|})/ where \s matches all space characters except
> \n.
> 6. Report the middle line the preceding regular expression matches to the user.
> 
> I'm confident I can write it as a shell script, but I don't know
> enough Perl to add that test to checkpatch.pl

That's basically what the $prevline variable in checkpatch does.
Likely it's enough to check that.
Perhaps Andy Whitcroft knows.

Re: [PATCH] staging: rtl8723au: Fixes unnecessary return warning

[Jes Sorensen]

Julian Calaby <julian.calaby@gmail.com> writes:
> Hi Bhaktipriya,
>
> On Sat, Jan 30, 2016 at 5:00 AM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
>> Bhaktipriya Shridhar <bhaktipriya96@gmail.com> writes:
>> If you insist on pushing this rather unncessary change, please do it
>> properly, and remove the blank line before the return statement as well.
>
> As Jes said, you need to remove the blank lines before the returns
> too. checkpatch should have picked this up, you did run the patch
> through checkpatch before you sent it, right?
>
> Jes,
>
> I know you have strong feelings on coding style, but there are a lot
> of people out there who see deviations from the standard as bugs to be
> fixed, so stuff like this isn't going to stop until it matches the
> coding style document's spec.

Julian,

rtl8723au is pretty dead development wise, so I don't care too much.
checkpatch is broken and has effectively turned into a policing tool for
a few people who wish to apply their narrow view onto everyone else.
I'll continue top reject broken patches to my code pushed out under
those rules.

Maybe it's time to introduce checkpatchconsideredharmful.com

Jes