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

From: Joe Perches
Date: Fri Jan 29 2016 - 22:19:10 EST


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@xxxxxxxxxxx>
> 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@xxxxxxxxx> 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