All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: wfx: sta: Fix 'else' coding style warning
@ 2021-09-14 14:31 Srivathsa Dara
  2021-09-14 14:56 ` Dan Carpenter
  2021-09-17 14:33 ` Jérôme Pouiller
  0 siblings, 2 replies; 5+ messages in thread
From: Srivathsa Dara @ 2021-09-14 14:31 UTC (permalink / raw)
  To: jerome.pouiller, gregkh; +Cc: linux-staging, linux-kernel, Srivathsa Dara

Fix 'else is not generally useful after a break or return' checkpatch
warning

Signed-off-by: Srivathsa Dara <srivathsa729.8@gmail.com>
---
 drivers/staging/wfx/sta.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index aceb18a1f54b..23c0425e3929 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -169,19 +169,18 @@ static int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
 			if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
 				dev_info(wvif->wdev->dev, "ignoring requested PS mode");
 			return -1;
-		} else {
-			/* It is necessary to enable PS if channels
-			 * are different.
-			 */
-			if (enable_ps)
-				*enable_ps = true;
-			if (wvif->wdev->force_ps_timeout > -1)
-				return wvif->wdev->force_ps_timeout;
-			else if (wfx_api_older_than(wvif->wdev, 3, 2))
-				return 0;
-			else
-				return 30;
 		}
+		/* It is necessary to enable PS if channels
+		 * are different.
+		 */
+		if (enable_ps)
+			*enable_ps = true;
+		if (wvif->wdev->force_ps_timeout > -1)
+			return wvif->wdev->force_ps_timeout;
+		else if (wfx_api_older_than(wvif->wdev, 3, 2))
+			return 0;
+		else
+			return 30;
 	}
 	if (enable_ps)
 		*enable_ps = wvif->vif->bss_conf.ps;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: wfx: sta: Fix 'else' coding style warning
  2021-09-14 14:31 [PATCH] staging: wfx: sta: Fix 'else' coding style warning Srivathsa Dara
@ 2021-09-14 14:56 ` Dan Carpenter
  2021-09-17 14:24   ` Greg KH
  2021-09-17 14:33 ` Jérôme Pouiller
  1 sibling, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2021-09-14 14:56 UTC (permalink / raw)
  To: Srivathsa Dara; +Cc: jerome.pouiller, gregkh, linux-staging, linux-kernel

On Tue, Sep 14, 2021 at 08:01:06PM +0530, Srivathsa Dara wrote:
> Fix 'else is not generally useful after a break or return' checkpatch
> warning
> 
> Signed-off-by: Srivathsa Dara <srivathsa729.8@gmail.com>

It doesn't apply for me.  Please check that you are working against
staging-next or linux-next.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: wfx: sta: Fix 'else' coding style warning
  2021-09-14 14:56 ` Dan Carpenter
@ 2021-09-17 14:24   ` Greg KH
  0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2021-09-17 14:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Srivathsa Dara, jerome.pouiller, linux-staging, linux-kernel

On Tue, Sep 14, 2021 at 05:56:43PM +0300, Dan Carpenter wrote:
> On Tue, Sep 14, 2021 at 08:01:06PM +0530, Srivathsa Dara wrote:
> > Fix 'else is not generally useful after a break or return' checkpatch
> > warning
> > 
> > Signed-off-by: Srivathsa Dara <srivathsa729.8@gmail.com>
> 
> It doesn't apply for me.  Please check that you are working against
> staging-next or linux-next.

It applied for me, odd.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: wfx: sta: Fix 'else' coding style warning
  2021-09-14 14:31 [PATCH] staging: wfx: sta: Fix 'else' coding style warning Srivathsa Dara
  2021-09-14 14:56 ` Dan Carpenter
@ 2021-09-17 14:33 ` Jérôme Pouiller
  2021-09-17 14:59   ` Greg KH
  1 sibling, 1 reply; 5+ messages in thread
From: Jérôme Pouiller @ 2021-09-17 14:33 UTC (permalink / raw)
  To: gregkh, linux-staging; +Cc: linux-kernel, Srivathsa Dara

Hello Srivathsa,

Thank for your suggestion. However ...

On Tuesday 14 September 2021 16:31:06 CEST Srivathsa Dara wrote:
> Fix 'else is not generally useful after a break or return' checkpatch
> warning
> 
> Signed-off-by: Srivathsa Dara <srivathsa729.8@gmail.com>
> ---
>  drivers/staging/wfx/sta.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> index aceb18a1f54b..23c0425e3929 100644
> --- a/drivers/staging/wfx/sta.c
> +++ b/drivers/staging/wfx/sta.c
> @@ -169,19 +169,18 @@ static int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
>                         if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
>                                 dev_info(wvif->wdev->dev, "ignoring requested PS mode");
>                         return -1;
> -               } else {
> -                       /* It is necessary to enable PS if channels
> -                        * are different.
> -                        */
> -                       if (enable_ps)
> -                               *enable_ps = true;
> -                       if (wvif->wdev->force_ps_timeout > -1)
> -                               return wvif->wdev->force_ps_timeout;
> -                       else if (wfx_api_older_than(wvif->wdev, 3, 2))
> -                               return 0;
> -                       else
> -                               return 30;
>                 }
> +               /* It is necessary to enable PS if channels
> +                * are different.
> +                */
> +               if (enable_ps)
> +                       *enable_ps = true;
> +               if (wvif->wdev->force_ps_timeout > -1)
> +                       return wvif->wdev->force_ps_timeout;
> +               else if (wfx_api_older_than(wvif->wdev, 3, 2))
> +                       return 0;
> +               else
> +                       return 30;

I am not a big fan of blindly applying the hints from checkpatch. With
this patch, it seems that the code in the "if" branch is an exception
and the rest of the code is the general case.

But, it is not true. There are two cases, and the author (me in fact)
attended to express that that by using a "else" statement.


-- 
Jérôme Pouiller



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: wfx: sta: Fix 'else' coding style warning
  2021-09-17 14:33 ` Jérôme Pouiller
@ 2021-09-17 14:59   ` Greg KH
  0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2021-09-17 14:59 UTC (permalink / raw)
  To: Jérôme Pouiller; +Cc: linux-staging, linux-kernel, Srivathsa Dara

On Fri, Sep 17, 2021 at 04:33:50PM +0200, Jérôme Pouiller wrote:
> Hello Srivathsa,
> 
> Thank for your suggestion. However ...
> 
> On Tuesday 14 September 2021 16:31:06 CEST Srivathsa Dara wrote:
> > Fix 'else is not generally useful after a break or return' checkpatch
> > warning
> > 
> > Signed-off-by: Srivathsa Dara <srivathsa729.8@gmail.com>
> > ---
> >  drivers/staging/wfx/sta.c | 23 +++++++++++------------
> >  1 file changed, 11 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> > index aceb18a1f54b..23c0425e3929 100644
> > --- a/drivers/staging/wfx/sta.c
> > +++ b/drivers/staging/wfx/sta.c
> > @@ -169,19 +169,18 @@ static int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
> >                         if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
> >                                 dev_info(wvif->wdev->dev, "ignoring requested PS mode");
> >                         return -1;
> > -               } else {
> > -                       /* It is necessary to enable PS if channels
> > -                        * are different.
> > -                        */
> > -                       if (enable_ps)
> > -                               *enable_ps = true;
> > -                       if (wvif->wdev->force_ps_timeout > -1)
> > -                               return wvif->wdev->force_ps_timeout;
> > -                       else if (wfx_api_older_than(wvif->wdev, 3, 2))
> > -                               return 0;
> > -                       else
> > -                               return 30;
> >                 }
> > +               /* It is necessary to enable PS if channels
> > +                * are different.
> > +                */
> > +               if (enable_ps)
> > +                       *enable_ps = true;
> > +               if (wvif->wdev->force_ps_timeout > -1)
> > +                       return wvif->wdev->force_ps_timeout;
> > +               else if (wfx_api_older_than(wvif->wdev, 3, 2))
> > +                       return 0;
> > +               else
> > +                       return 30;
> 
> I am not a big fan of blindly applying the hints from checkpatch. With
> this patch, it seems that the code in the "if" branch is an exception
> and the rest of the code is the general case.
> 
> But, it is not true. There are two cases, and the author (me in fact)
> attended to express that that by using a "else" statement.

But that first part of the if statement returns, making the second part
not needed to be indented at all as the code flow just moves on here.

It's best not to indent if not needed, and it's not needed here as
the tools are saying.  So I've applied this.

thanks,

greg k-h


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-17 14:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14 14:31 [PATCH] staging: wfx: sta: Fix 'else' coding style warning Srivathsa Dara
2021-09-14 14:56 ` Dan Carpenter
2021-09-17 14:24   ` Greg KH
2021-09-17 14:33 ` Jérôme Pouiller
2021-09-17 14:59   ` Greg KH

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.