public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Joshua Clayton <stillcompiling@gmail.com>
To: punit vara <punitvara@gmail.com>
Cc: Larry Finger <Larry.Finger@lwfinger.net>,
	Flo Schil <florian.c.schilhabel@googlemail.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Sudip Mukherjee <sudipm.mukherjee@gmail.com>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	dogukan.ergun@gmail.com, Joe Perches <joe@perches.com>
Subject: Re: [PATCH] Staging: rtl8712: rtl871x_ioctl_linux.c Move constant to right side of the comparision
Date: Sat, 26 Sep 2015 11:11:56 -0700	[thread overview]
Message-ID: <1477010.B1YWVeUsfC@diplodocus> (raw)
In-Reply-To: <CABXAfNLJv875fWG4xL2NnWpttweR2bqgUVX1wgt5xQLzarSEDw@mail.gmail.com>

On Saturday, September 26, 2015 11:24:06 PM punit vara wrote:
> On Sat, Sep 26, 2015 at 11:15 PM, Larry Finger
> <Larry.Finger@lwfinger.net> wrote:
> > On 09/26/2015 11:49 AM, Punit Vara wrote:
> >>
> >> This patch is to the rtl871x_ioctl_linux.c that fixes up following
> >> warning reported by checkpatch.pl :
> >>
> >> - Comparisons should place the constant on the right side of the test
> >>
> >> Signed-off-by: Punit Vara <punitvara@gmail.com>
> >
> >
> > This warning is crap. WTF difference does it make???? The compiler does not
> > care, and any reader with any piece of a brain is not going to be confused!
> >
> > This patch and all others like it are just meaningless source churning!
> >
> > This author has made such a royal mess of his patches that I recommend that
> > ALL of them be dropped. In addition, we should continue to drop his changes
> > until he learns how to use git to generate N/M patches, and until he reads
> > the documentation on patch submission.
> >
> > Larry
> >
> >
> >> ---
> >>   drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 16 ++++++++--------
> >>   1 file changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> index 143be0f..6d7f8f7 100644
> >> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> >> @@ -957,7 +957,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
> >>         if (IS_ERR(ext))
> >>                 return PTR_ERR(ext);
> >>
> >> -       if (0 == strcasecmp(ext, "RSSI")) {
> >> +       if (strcasecmp(ext, "RSSI") == 0) {
> >>                 /*Return received signal strength indicator in -db for */
> >>                 /* current AP */
> >>                 /*<ssid> Rssi xx */
> >> @@ -974,7 +974,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
> >>                 } else {
> >>                         sprintf(ext, "OK");
> >>                 }
> >> -       } else if (0 == strcasecmp(ext, "LINKSPEED")) {
> >> +       } else if (strcasecmp(ext, "LINKSPEED") == 0) {
> >>                 /*Return link speed in MBPS */
> >>                 /*LinkSpeed xx */
> >>                 union iwreq_data wrqd;
> >> @@ -982,30 +982,30 @@ static int r871x_wx_set_priv(struct net_device *dev,
> >>                 int mbps;
> >>
> >>                 ret_inner = r8711_wx_get_rate(dev, info, &wrqd, extra);
> >> -               if (0 != ret_inner)
> >> +               if (ret_inner != 0)
> >>                         mbps = 0;
> >>                 else
> >>                         mbps = wrqd.bitrate.value / 1000000;
> >>                 sprintf(ext, "LINKSPEED %d", mbps);
> >> -       } else if (0 == strcasecmp(ext, "MACADDR")) {
> >> +       } else if (strcasecmp(ext, "MACADDR") == 0) {
> >>                 /*Return mac address of the station */
> >>                 /* Macaddr = xx:xx:xx:xx:xx:xx */
> >>                 sprintf(ext, "MACADDR = %pM", dev->dev_addr);
> >> -       } else if (0 == strcasecmp(ext, "SCAN-ACTIVE")) {
> >> +       } else if (strcasecmp(ext, "SCAN-ACTIVE") == 0) {
> >>                 /*Set scan type to active */
> >>                 /*OK if successful */
> >>                 struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> >>
> >>                 pmlmepriv->passive_mode = 1;
> >>                 sprintf(ext, "OK");
> >> -       } else if (0 == strcasecmp(ext, "SCAN-PASSIVE")) {
> >> +       } else if (strcasecmp(ext, "SCAN-PASSIVE") == 0) {
> >>                 /*Set scan type to passive */
> >>                 /*OK if successful */
> >>                 struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> >>
> >>                 pmlmepriv->passive_mode = 0;
> >>                 sprintf(ext, "OK");
> >> -       } else if (0 == strncmp(ext, "DCE-E", 5)) {
> >> +       } else if (strncmp(ext, "DCE-E", 5) == 0) {
> >>                 /*Set scan type to passive */
> >>                 /*OK if successful */
> >>                 r8712_disconnectCtrlEx_cmd(padapter
> >> @@ -1015,7 +1015,7 @@ static int r871x_wx_set_priv(struct net_device *dev,
> >>                         , 5000 /*u32 firstStageTO */
> >>                 );
> >>                 sprintf(ext, "OK");
> >> -       } else if (0 == strncmp(ext, "DCE-D", 5)) {
> >> +       } else if (strncmp(ext, "DCE-D", 5) == 0) {
> >>                 /*Set scan type to passive */
> >>                 /*OK if successfu */
> >>                 r8712_disconnectCtrlEx_cmd(padapter
> >>
> >
> 
> Sorry I have disappointed you .I learned now how to generate N/M
> patches .I have recently sent my last 6 patches in that way only.   I
> am extremely sorry for previous patches as I could not able to send in
> series. Please let me allow to send me those patches again in correct
> way. If you would say I will definitely send them in series.Can I send
> all the patches in series for rtl8712 directory if you do not mind ?
I have not seen your other patches, but I agree with Larry. This is a formatting patch
that  does not improve the readability of the code. Should be dropped.
This particular checkpatch warning might be useful in expressing a preference for new code, I guess.

  reply	other threads:[~2015-09-26 18:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-26 16:49 [PATCH] Staging: rtl8712: rtl871x_ioctl_linux.c Move constant to right side of the comparision Punit Vara
2015-09-26 17:45 ` Larry Finger
2015-09-26 17:54   ` punit vara
2015-09-26 18:11     ` Joshua Clayton [this message]
2015-09-28  8:35     ` Dan Carpenter
2015-09-28 11:18   ` Sudip Mukherjee
2015-09-28 13:46     ` punit vara
2015-09-28 14:56     ` Larry Finger
2015-09-28 15:19       ` Sudip Mukherjee
2015-09-29 20:24       ` Tillmann Heidsieck
2015-09-30  9:34         ` Dan Carpenter
2015-10-09 14:54           ` Dan Carpenter

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=1477010.B1YWVeUsfC@diplodocus \
    --to=stillcompiling@gmail.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=dogukan.ergun@gmail.com \
    --cc=florian.c.schilhabel@googlemail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=punitvara@gmail.com \
    --cc=sudipm.mukherjee@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox