Prev: drivers/staging/vt6656: Hoist assign from if
Next: trivial: use (C) instead of \251
From: Greg KH on 28 Apr 2010 14:40
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?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Joe Perches on 28 Apr 2010 15:10
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

>From an earlier Greg KH email:

> Careful that you don't add new coding style issues to a staging driver

Avoiding checkpatch errors and avoiding new coding style issues
are frequently conflicting goals.

I try not to add "new" issues. and I generally
leave the old ones alone.

I don't use checkpatch as a guide for what to fix
or as an inhibitor on what style patch to send.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 28 Apr 2010 15:20
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.

Oh well, I did it by hand for you...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Joe Perches on 28 Apr 2010 15:30
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 28 Apr 2010 15:50
On Wed, Apr 28, 2010 at 12:20:46PM -0700, Joe Perches wrote:
> 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.

"I know it when I see it."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2
Prev: drivers/staging/vt6656: Hoist assign from if
Next: trivial: use (C) instead of \251