linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] wifi: ray_cs: Remove invalid conditional statements
@ 2023-06-26 10:27 You Kangren
  2023-06-26 12:40 ` Simon Horman
  2023-06-26 13:06 ` Julian Calaby
  0 siblings, 2 replies; 4+ messages in thread
From: You Kangren @ 2023-06-26 10:27 UTC (permalink / raw)
  To: Kalle Valo, Dongliang Mu, You Kangren, Simon Horman,
	Christophe JAILLET,
	open list:RAYLINK/WEBGEAR 802.11 WIRELESS LAN DRIVER, open list
  Cc: opensource.kernel

Remove invalid conditional statements to make the code clean

Signed-off-by: You Kangren <youkangren@vivo.com>
---
 drivers/net/wireless/legacy/ray_cs.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/net/wireless/legacy/ray_cs.c b/drivers/net/wireless/legacy/ray_cs.c
index 96f34d90f601..0022038b0758 100644
--- a/drivers/net/wireless/legacy/ray_cs.c
+++ b/drivers/net/wireless/legacy/ray_cs.c
@@ -2116,7 +2116,6 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
 #endif
 
 	if (!sniffer) {
-		if (translate) {
 /* TBD length needs fixing for translated header */
 			if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
 			    rx_len >
@@ -2126,18 +2125,6 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
 				      "ray_cs invalid packet length %d received\n",
 				      rx_len);
 				return;
-			}
-		} else { /* encapsulated ethernet */
-
-			if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
-			    rx_len >
-			    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
-			     FCS_LEN)) {
-				pr_debug(
-				      "ray_cs invalid packet length %d received\n",
-				      rx_len);
-				return;
-			}
 		}
 	}
 	pr_debug("ray_cs rx_data packet\n");
-- 
2.39.0


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

* Re: [PATCH] wifi: ray_cs: Remove invalid conditional statements
  2023-06-26 10:27 [PATCH] wifi: ray_cs: Remove invalid conditional statements You Kangren
@ 2023-06-26 12:40 ` Simon Horman
  2023-06-26 13:06 ` Julian Calaby
  1 sibling, 0 replies; 4+ messages in thread
From: Simon Horman @ 2023-06-26 12:40 UTC (permalink / raw)
  To: You Kangren
  Cc: Kalle Valo, Dongliang Mu, Christophe JAILLET,
	open list:RAYLINK/WEBGEAR 802.11 WIRELESS LAN DRIVER, open list,
	opensource.kernel

On Mon, Jun 26, 2023 at 06:27:50PM +0800, You Kangren wrote:
> Remove invalid conditional statements to make the code clean
>
> Signed-off-by: You Kangren <youkangren@vivo.com>
> ---
>  drivers/net/wireless/legacy/ray_cs.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/net/wireless/legacy/ray_cs.c b/drivers/net/wireless/legacy/ray_cs.c
> index 96f34d90f601..0022038b0758 100644
> --- a/drivers/net/wireless/legacy/ray_cs.c
> +++ b/drivers/net/wireless/legacy/ray_cs.c
> @@ -2116,7 +2116,6 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
>  #endif
> 
>         if (!sniffer) {
> -               if (translate) {
>  /* TBD length needs fixing for translated header */
>                         if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
>                             rx_len >
> @@ -2126,18 +2125,6 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
>                                       "ray_cs invalid packet length %d received\n",
>                                       rx_len);
>                                 return;
> -                       }

Hi You Kangren,

Some minor nits from my side:

1. The indentation of the code is now wrong,
   it should be adjusted one tab-stop to the left.

2. The two remaining conditions can be collapsed,
   which leads to a further reduction in indentation.

3. Also, the indentation of the "TBD" comment was wrong before this patch,
   and is still wrong. So we can probably fix that while we are here.

I suggest folding in something like this (completely untested!).

diff --git a/drivers/net/wireless/legacy/ray_cs.c b/drivers/net/wireless/legacy/ray_cs.c
index f0bc7a06a257..f339927f1f56 100644
--- a/drivers/net/wireless/legacy/ray_cs.c
+++ b/drivers/net/wireless/legacy/ray_cs.c
@@ -2116,17 +2116,12 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
 	u_char linksrcaddr[ETH_ALEN];	/* Other end of the wireless link */
 #endif
 
-	if (!sniffer) {
-/* TBD length needs fixing for translated header */
-			if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
-			    rx_len >
-			    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
-			     FCS_LEN)) {
-				pr_debug(
-				      "ray_cs invalid packet length %d received\n",
-				      rx_len);
-				return;
-		}
+	/* TBD length needs fixing for translated header */
+	if (!sniffer && (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
+		         rx_len > (dev->mtu + RX_MAC_HEADER_LENGTH +
+				   ETH_HLEN + FCS_LEN))) {
+		pr_debug("ray_cs invalid packet length %d received\n", rx_len);
+		return;
 	}
 	pr_debug("ray_cs rx_data packet\n");
 	/* If fragmented packet, verify sizes of fragments add up */

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

* Re: [PATCH] wifi: ray_cs: Remove invalid conditional statements
  2023-06-26 10:27 [PATCH] wifi: ray_cs: Remove invalid conditional statements You Kangren
  2023-06-26 12:40 ` Simon Horman
@ 2023-06-26 13:06 ` Julian Calaby
       [not found]   ` <PUZPR06MB59369ADE01FB04AD9A7279A9AA27A@PUZPR06MB5936.apcprd06.prod.outlook.com>
  1 sibling, 1 reply; 4+ messages in thread
From: Julian Calaby @ 2023-06-26 13:06 UTC (permalink / raw)
  To: You Kangren
  Cc: Kalle Valo, Dongliang Mu, Simon Horman, Christophe JAILLET,
	open list:RAYLINK/WEBGEAR 802.11 WIRELESS LAN DRIVER, open list,
	opensource.kernel

Hi,

On Mon, Jun 26, 2023 at 8:36 PM You Kangren <youkangren@vivo.com> wrote:
>
> Remove invalid conditional statements to make the code clean
>
> Signed-off-by: You Kangren <youkangren@vivo.com>

"to make the code clean" isn't enough to explain why this change might
be wanted. Are both branches the same? Is there a compiler warning?
How did you find this? etc.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

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

* Re: [PATCH] wifi: ray_cs: Remove invalid conditional statements
       [not found]   ` <PUZPR06MB59369ADE01FB04AD9A7279A9AA27A@PUZPR06MB5936.apcprd06.prod.outlook.com>
@ 2023-06-27 12:58     ` Julian Calaby
  0 siblings, 0 replies; 4+ messages in thread
From: Julian Calaby @ 2023-06-27 12:58 UTC (permalink / raw)
  To: 尤康仁
  Cc: Kalle Valo, Dongliang Mu, Simon Horman, Christophe JAILLET,
	open list:RAYLINK/WEBGEAR 802.11 WIRELESS LAN DRIVER, open list,
	opensource.kernel

Hi,

On Tue, Jun 27, 2023 at 10:44 PM 尤康仁 <youkangren@vivo.com> wrote:
>
> Dear Julian,
>
>     I found a warning at compiling time that the if and else branches had no effect here. I looked at the code and found that the contents of both branches were the same, so I merged the contents of both branches into one and committed the changes.

That's great, but it needs to be in the commit message.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

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

end of thread, other threads:[~2023-06-27 12:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-26 10:27 [PATCH] wifi: ray_cs: Remove invalid conditional statements You Kangren
2023-06-26 12:40 ` Simon Horman
2023-06-26 13:06 ` Julian Calaby
     [not found]   ` <PUZPR06MB59369ADE01FB04AD9A7279A9AA27A@PUZPR06MB5936.apcprd06.prod.outlook.com>
2023-06-27 12:58     ` Julian Calaby

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).