Re: [PATCH 09/13] drivers/staging/rtl8192su: Hoist assign from if

[Joe Perches <joe@perches.com>]

On Wed, 2010-04-28 at 12:11 -0700, Greg KH wrote:
> On Wed, Apr 28, 2010 at 12:04:49PM -0700, Joe Perches wrote:
> > On Wed, 2010-04-28 at 11:27 -0700, Greg KH wrote:
> > > On Wed, Mar 24, 2010 at 10:17:03PM -0700, Joe Perches wrote:
> > > > --- a/drivers/staging/rtl8192su/ieee80211/ieee80211_softmac.c
> > > > +++ b/drivers/staging/rtl8192su/ieee80211/ieee80211_softmac.c
> > > > @@ -1690,7 +1690,8 @@ ieee80211_rx_auth_rq(struct ieee80211_device *ieee, struct sk_buff *skb)
> > > >  	//IEEE80211DMESG("Rx probe");
> > > >  	ieee->softmac_stats.rx_auth_rq++;
> > > >  
> > > > -	if ((status = auth_rq_parse(skb, dest))!= -1){
> > > > +	status = auth_rq_parse(skb, dest);
> > > > +	if (status!= -1) {
> > > 
> > > And again.  Did you do this with some tool and not by hand?
> > []
> > > Did you not run your patch through checkpatch after creating it and 
> > > before sending it to me?
> > 
> > >From the original 0/13 introduction:
> > http://lkml.org/lkml/2010/3/25/6
> > 
> >         Used scripts/cvt_kernel.style.pl:
> >         --convert-hoist_assigns_from_if
> >         and verified visually.
> >         
> >         checkpatch errors ignored
> > 
> > The script itself is:
> > http://lkml.org/lkml/2010/3/24/447
> 
> And this points out why I hate scripts.
> 
> It is _trivial_ to fix up the obvious formatting problem, on the same
> line that you are modifying, when doing so.

Define _trivial_ and define obvious.

Here you've multiple situations of initial line spacing
not using tabs, odd use of parentheses in the original
style, compressed logical tests or assigns, etc.

All trivially fixable, but not necessarily correct to do
in a single pass.

If you do them in a single pass, you run into your "do too
much in a single patch" argument.

cheers, Joe