All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: "Frank A. Cancio Bello" <frank@generalsoftwareinc.com>
Cc: gregkh@linuxfoundation.org, Julia.Lawall@lip6.fr,
	pablo@netfilter.org, devel@driverdev.osuosl.org,
	outreachy-kernel@googlegroups.com
Subject: Re: [PATCH v3] staging: wlan-ng: Remove unnecessary parentheses
Date: Thu, 19 Oct 2017 12:38:16 +1100	[thread overview]
Message-ID: <20171019013816.GF31318@eros> (raw)
In-Reply-To: <20171018225052.GA20395@ubuntu-server-1604>

On Wed, Oct 18, 2017 at 06:50:52PM -0400, Frank A. Cancio Bello wrote:
> On Thu, Oct 19, 2017 at 08:40:08AM +1100, Tobin C. Harding wrote:
> > On Wed, Oct 18, 2017 at 11:48:21AM -0400, Frank A. Cancio Bello wrote:
> > > --- a/drivers/staging/wlan-ng/p80211req.c
> > > +++ b/drivers/staging/wlan-ng/p80211req.c
> > > @@ -124,7 +124,7 @@ int p80211req_dorequest(struct wlandevice *wlandev, u8 *msgbuf)
> > >  
> > >  	/* Check Permissions */
> > >  	if (!capable(CAP_NET_ADMIN) &&
> > > -	    (msg->msgcode != DIDmsg_dot11req_mibget)) {
> > > +	    msg->msgcode != DIDmsg_dot11req_mibget) {
> > 
> > While this is not making the code _harder_ to read, it is not making it any easier either. So all
> > the change is really doing is quieting checkpatch. Usually it is not a good idea to make code
> > changes _just_ to quieten a static analysis tool. It's just a tool remember, there to help us write
> > better code.
> > 
> 
> For me is easy to read without parentheses given the fact that I tend to jump to the closing parentheses and then read from the opening parentheses up to the mental mark that I did at the closing parentheses. But that is me, and given the fact that I'm a newbie that is still learning I will stop sending this kind of patches if you consider it wise.
> 
> > On top of that CHECKS are just that, things that should be CHECK'ed, not necessarily fixed.
> > 
> 
> Agreed.
> 
> > Hope this helps,
> 
> A lot! I really appreciate any input at this stage.

Glad to help, stick at it. You will get there.

> 
> thanks,
> frank


  reply	other threads:[~2017-10-19  1:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-16 18:26 [PATCH] staging: wlan-ng: Remove unnecessary parentheses Frank A. Cancio Bello
2017-10-16 20:18 ` [Outreachy kernel] " Julia Lawall
2017-10-16 21:45   ` Frank A. Cancio Bello
2017-10-16 21:47     ` Julia Lawall
2017-10-17  1:48 ` [PATCH v2] " Frank A. Cancio Bello
2017-10-18 14:17   ` Greg KH
2017-10-18 14:46     ` Frank A. Cancio Bello
2017-10-18 14:55       ` Greg KH
2017-10-18 15:48         ` [PATCH v3] " Frank A. Cancio Bello
2017-10-18 21:40           ` Tobin C. Harding
2017-10-18 22:50             ` Frank A. Cancio Bello
2017-10-19  1:38               ` Tobin C. Harding [this message]
2017-10-19 11:02   ` [PATCH v2] " kbuild test robot
2017-10-18 10:45 ` [PATCH] " kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171019013816.GF31318@eros \
    --to=me@tobin.cc \
    --cc=Julia.Lawall@lip6.fr \
    --cc=devel@driverdev.osuosl.org \
    --cc=frank@generalsoftwareinc.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=outreachy-kernel@googlegroups.com \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.