Message ID | Y2dvmdGxQfmK4O6F@qemulion |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp1391790wru; Sun, 6 Nov 2022 01:34:01 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6tUzjHhWKdA81ofKf3Av8x/vcQjxB0AjxMf97rX7Ytc/wnnWtGa0FifiAoNE6GnSZDxPm6 X-Received: by 2002:aa7:9421:0:b0:56b:b2a8:6822 with SMTP id y1-20020aa79421000000b0056bb2a86822mr43924395pfo.86.1667723640937; Sun, 06 Nov 2022 01:34:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667723640; cv=none; d=google.com; s=arc-20160816; b=W8/j+g9oPK7Kn4BCNn8nEt4a3+PEbSZeXOioW+WA4X/0EhEscT1BWzngLG2pB4c97K YJt6qp90ZdJa+nfI6y2fP+vlwyCM9hxWuZa4QTZvm0a/H/4wXCSl2vqSgTVIqBGRSNLt Oi49nMb8anACS8nw5pG/TAPvZ/JG2/4k0YhT4HYZ6PqqMcX0fRpyMuc9cIt1SoFMpGjx nLVqEMT9vMKQwjWPpEa/PFLsRQA8LThdkXFKgLmx3KKUPTBADLJsYJjrcSr1nK1y7r4Y jfY1D1DfOryUb3B5lJ5UyktJlYJdaYcvB6z45RI+01Dh6fSb63uR17VNcNgJzsPe1CYm AXLg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-disposition:mime-version:message-id :subject:to:from:date:dkim-signature; bh=l4GM1RCc6ewQ7dezsnZMGm/nrFZfzm+P55YyG6OSXZ8=; b=q9otlOO8vtskCnpzZhsrvvcqJO1PVd7UJHy9ZaQfMPOhQs2nmgdYVIm4GsnvNUlnQN vr9y5rDoCZxGakr99VZuMMnXNq61IYBe90OYoO9T7NKbB3/n/xo2Zyhy5mn+iaUry+RF 3uXmv3aPMRrzAr7stZkGN1ocMu10YSIq4BNOUsvy/QDEhc/Ou4pBRT+jVP/XNaKWDiMq 62E+47hX6zo1WlTlgt55TotWd/oBIDajyse78y1S6vD4YNalKg1h1h+MrPIFlsSZE/MN aeYtBDB6iWtKw4HhavurVB901apRroENaCq92YuzBk6Cz/oxk093Hg3Pm9Hx0AWWJqG2 YLWA== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@mailo.com header.s=mailo header.b=np7fFDM1; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id j30-20020a634a5e000000b00438894246e8si5211640pgl.169.2022.11.06.01.33.46; Sun, 06 Nov 2022 01:34:00 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@mailo.com header.s=mailo header.b=np7fFDM1; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=mailo.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229635AbiKFI0d (ORCPT <rfc822;hjfbswb@gmail.com> + 99 others); Sun, 6 Nov 2022 03:26:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58542 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229589AbiKFI0a (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 6 Nov 2022 03:26:30 -0500 Received: from msg-2.mailo.com (msg-2.mailo.com [213.182.54.12]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 609CCB1C0 for <linux-kernel@vger.kernel.org>; Sun, 6 Nov 2022 01:26:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=mailo.com; s=mailo; t=1667723169; bh=yX2+265rR785M95fcHNy467AL5AwbELaJnp6X7G2qZA=; h=X-EA-Auth:Date:From:To:Subject:Message-ID:MIME-Version: Content-Type; b=np7fFDM1XDP0KCPeEjakvsHzqo7RO/2yRlvxCCCNQ/6NLWYt/yv+kgZMJQoY/xfpZ aEmI+KvXRRYiuU+D/2wT3OWEiW/aTxiC2WhhczVn5eQSNkLa2sxvjthojgCDsCoA1Z g2eYVoeZTXjS5hhn74r7fJ5PDPfrGjo+qfCXcD+w= Received: by b-6.in.mailobj.net [192.168.90.16] with ESMTP via ip-206.mailobj.net [213.182.55.206] Sun, 6 Nov 2022 09:26:09 +0100 (CET) X-EA-Auth: XktWBgDFtHub354BC+dxxKliuilwtqXGDLzjuFBXbch+6Vu+T4PKfcqK1yl3KXCjd08T7ttmQsZpXyjbsVWn622SMiCd1gX+ Date: Sun, 6 Nov 2022 13:56:01 +0530 From: Deepak R Varma <drv@mailo.com> To: Larry Finger <Larry.Finger@lwfinger.net>, Phillip Potter <phil@philpotter.co.uk>, Pavel Skripkin <paskripkin@gmail.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [PATCH] staging: r8188eu: simplify complex pointer casting Message-ID: <Y2dvmdGxQfmK4O6F@qemulion> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748734984626034995?= X-GMAIL-MSGID: =?utf-8?q?1748734984626034995?= |
Series |
staging: r8188eu: simplify complex pointer casting
|
|
Commit Message
Deepak R Varma
Nov. 6, 2022, 8:26 a.m. UTC
Pointers to structures udphdr and dhcpMessage are derived by casting
adjacent pointers with size_t. Such typecast of pointer using size_t
is not preferred. The code looks complex and delicate. Simplify such
casting by utilizing generic "void *" casting.
While at this change, remove the unnecessary __be32 casting for member
variable "cookie".
Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Deepak R Varma <drv@mailo.com>
---
drivers/staging/r8188eu/core/rtw_br_ext.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--
2.34.1
Comments
On 11/6/22 09:26, Deepak R Varma wrote: > Pointers to structures udphdr and dhcpMessage are derived by casting > adjacent pointers with size_t. Such typecast of pointer using size_t > is not preferred. The code looks complex and delicate. Simplify such > casting by utilizing generic "void *" casting. > While at this change, remove the unnecessary __be32 casting for member > variable "cookie". > > Suggested-by: Joe Perches <joe@perches.com> > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > drivers/staging/r8188eu/core/rtw_br_ext.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c > index a23f7df373ed..e9b0906d0d74 100644 > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c > @@ -610,13 +610,15 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb) > struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN); > > if (iph->protocol == IPPROTO_UDP) { /* UDP */ > - struct udphdr *udph = (struct udphdr *)((size_t)iph + (iph->ihl << 2)); > + struct udphdr *udph = (void *)iph + (iph->ihl << 2); > > if ((udph->source == htons(CLIENT_PORT)) && > (udph->dest == htons(SERVER_PORT))) { /* DHCP request */ > - struct dhcpMessage *dhcph = > - (struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr)); > - u32 cookie = be32_to_cpu((__be32)dhcph->cookie); > + u32 cookie; > + struct dhcpMessage *dhcph; > + > + dhcph = (void *)udph + sizeof(struct udphdr); > + cookie = be32_to_cpu(dhcph->cookie); > > if (cookie == DHCP_MAGIC) { /* match magic word */ > if (!(dhcph->flags & htons(BROADCAST_FLAG))) { > -- > 2.34.1 > > > > Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150
On Sun, Nov 06, 2022 at 01:56:01PM +0530, Deepak R Varma wrote: > Pointers to structures udphdr and dhcpMessage are derived by casting > adjacent pointers with size_t. Such typecast of pointer using size_t > is not preferred. The code looks complex and delicate. Simplify such > casting by utilizing generic "void *" casting. > While at this change, remove the unnecessary __be32 casting for member > variable "cookie". Wait, why is that unecessary? And why is that part of this change, that should be a separate one, so that it can be reverted when it's found to be buggy :) > > Suggested-by: Joe Perches <joe@perches.com> > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > drivers/staging/r8188eu/core/rtw_br_ext.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c > index a23f7df373ed..e9b0906d0d74 100644 > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c > @@ -610,13 +610,15 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb) > struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN); > > if (iph->protocol == IPPROTO_UDP) { /* UDP */ > - struct udphdr *udph = (struct udphdr *)((size_t)iph + (iph->ihl << 2)); > + struct udphdr *udph = (void *)iph + (iph->ihl << 2); > > if ((udph->source == htons(CLIENT_PORT)) && > (udph->dest == htons(SERVER_PORT))) { /* DHCP request */ > - struct dhcpMessage *dhcph = > - (struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr)); > - u32 cookie = be32_to_cpu((__be32)dhcph->cookie); > + u32 cookie; > + struct dhcpMessage *dhcph; Why reorder these variables? > + > + dhcph = (void *)udph + sizeof(struct udphdr); > + cookie = be32_to_cpu(dhcph->cookie); The cookie change should be separate please. thanks, greg k-h
On Sun, Nov 06, 2022 at 05:43:38PM +0100, Greg Kroah-Hartman wrote: > On Sun, Nov 06, 2022 at 01:56:01PM +0530, Deepak R Varma wrote: > > Pointers to structures udphdr and dhcpMessage are derived by casting > > adjacent pointers with size_t. Such typecast of pointer using size_t > > is not preferred. The code looks complex and delicate. Simplify such > > casting by utilizing generic "void *" casting. > > While at this change, remove the unnecessary __be32 casting for member > > variable "cookie". > > Wait, why is that unecessary? Hello Greg, cookie is already defined to be a __be32 type as part of the dhcpMesssade structure declared in the same file. 15 struct dhcpMessage { 14 u_int8_t op; 1 u_int8_t file[128]; 597 __be32 cookie; 1 u_int8_t options[308]; /* 312 - cookie */ 2 }; 3 > > And why is that part of this change, that should be a separate one, so > that it can be reverted when it's found to be buggy :) Sounds good. I will submit that as a separate patch. > > > > > Suggested-by: Joe Perches <joe@perches.com> > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > --- > > drivers/staging/r8188eu/core/rtw_br_ext.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c > > index a23f7df373ed..e9b0906d0d74 100644 > > --- a/drivers/staging/r8188eu/core/rtw_br_ext.c > > +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c > > @@ -610,13 +610,15 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb) > > struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN); > > > > if (iph->protocol == IPPROTO_UDP) { /* UDP */ > > - struct udphdr *udph = (struct udphdr *)((size_t)iph + (iph->ihl << 2)); > > + struct udphdr *udph = (void *)iph + (iph->ihl << 2); > > > > if ((udph->source == htons(CLIENT_PORT)) && > > (udph->dest == htons(SERVER_PORT))) { /* DHCP request */ > > - struct dhcpMessage *dhcph = > > - (struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr)); > > - u32 cookie = be32_to_cpu((__be32)dhcph->cookie); > > + u32 cookie; > > + struct dhcpMessage *dhcph; > > Why reorder these variables? No specific reason. Since next instruction is associated with *dhcph, I thought it will simpler to follow. I will revise and resend. > > > + > > + dhcph = (void *)udph + sizeof(struct udphdr); > > + cookie = be32_to_cpu(dhcph->cookie); > > The cookie change should be separate please. Sounds good. Will split the patch into a patch set and resubmit. > > thanks, > > greg k-h >
diff --git a/drivers/staging/r8188eu/core/rtw_br_ext.c b/drivers/staging/r8188eu/core/rtw_br_ext.c index a23f7df373ed..e9b0906d0d74 100644 --- a/drivers/staging/r8188eu/core/rtw_br_ext.c +++ b/drivers/staging/r8188eu/core/rtw_br_ext.c @@ -610,13 +610,15 @@ void dhcp_flag_bcast(struct adapter *priv, struct sk_buff *skb) struct iphdr *iph = (struct iphdr *)(skb->data + ETH_HLEN); if (iph->protocol == IPPROTO_UDP) { /* UDP */ - struct udphdr *udph = (struct udphdr *)((size_t)iph + (iph->ihl << 2)); + struct udphdr *udph = (void *)iph + (iph->ihl << 2); if ((udph->source == htons(CLIENT_PORT)) && (udph->dest == htons(SERVER_PORT))) { /* DHCP request */ - struct dhcpMessage *dhcph = - (struct dhcpMessage *)((size_t)udph + sizeof(struct udphdr)); - u32 cookie = be32_to_cpu((__be32)dhcph->cookie); + u32 cookie; + struct dhcpMessage *dhcph; + + dhcph = (void *)udph + sizeof(struct udphdr); + cookie = be32_to_cpu(dhcph->cookie); if (cookie == DHCP_MAGIC) { /* match magic word */ if (!(dhcph->flags & htons(BROADCAST_FLAG))) {