2019-07-20 18:50:02

by Hariprasad Kelam

Subject: [PATCH] rat_cs: Remove duplicate code

Code is same if translate is true/false in case invalid packet is
received.So remove else part.

Issue identified with coccicheck

Signed-off-by: Hariprasad Kelam <[email protected]>
---
drivers/net/wireless/ray_cs.c | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index cf37268..a51bbe7 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -2108,29 +2108,16 @@ 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 >
- (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
- FCS_LEN)) {
- pr_debug(
- "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;
+ 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");
/* If fragmented packet, verify sizes of fragments add up */
--
2.7.4

2019-07-21 09:13:33

by Stefano Brivio

Subject: Re: [PATCH] rat_cs: Remove duplicate code

On Sat, 20 Jul 2019 23:16:47 +0530
Hariprasad Kelam <[email protected]> wrote:

> Code is same if translate is true/false in case invalid packet is
> received.So remove else part.
>
> Issue identified with coccicheck
>
> Signed-off-by: Hariprasad Kelam <[email protected]>
> ---
> drivers/net/wireless/ray_cs.c | 29 ++++++++---------------------
> 1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
> index cf37268..a51bbe7 100644
> --- a/drivers/net/wireless/ray_cs.c
> +++ b/drivers/net/wireless/ray_cs.c
> @@ -2108,29 +2108,16 @@ 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 >
> - (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
> - FCS_LEN)) {
> - pr_debug(
> - "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;
> + 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;
> }
> - }

NACK. The TBD comment makes no sense anymore if you remove one of the
branches. Believe me or not, I have one of those cards, a (yes, 22
years old) Buslink Raytheon model 24020. That check needed (for sure)
and needs (maybe) to be fixed.

Besides, patch subject and resulting coding style are also wrong.

--
Stefano