[1/2,next] scsi: qla2xxx: Replace one-element array with DECLARE_FLEX_ARRAY() helper
Message ID | faa40e6b31ecc9387ad1644bb1957aa53d7c682f.1668814746.git.gustavoars@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp488736wrr; Fri, 18 Nov 2022 16:46:55 -0800 (PST) X-Google-Smtp-Source: AA0mqf68eXiFRNdIOxd85kEwcEV1IdwiPV9a4rI1OW5kuHWFQXBV7lcDCF401fUDNhyKJFPRrkfz X-Received: by 2002:a17:906:1b15:b0:7ad:dc7e:1b8d with SMTP id o21-20020a1709061b1500b007addc7e1b8dmr7827474ejg.276.1668818814903; Fri, 18 Nov 2022 16:46:54 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668818814; cv=none; d=google.com; s=arc-20160816; b=Z36KcVv+59iqblu6B2bg9g8uIx76GQKG81joc6JZb5K/AF2aYQiHJXo/zHhU0bnbmG jck+bi1luchbKhl83ihCw0IcG7U7ka6uXeyNE7YhEwzUnVcdmOk3x9afqwkiHiUhRpIQ tLaHTi12HuSoeg/LPFJrHKDV2kU8QPifp0czSopzADJ2nDwHudRtmJL4VFYowaSEapU4 GqBoKrIqNnxk3/A4MQPfoc/CckNrqScnT2JE0GLSH2w/aK2fOO/xFgMjuONwXg3ouIpi PnSBUsidgYsYwsiAwR/HKPS9QPz5IA5CsPlSCNkTn2nbq3J48L4nVLFv6/hB/DIczqLG dJFA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=PFWdbihZk7UEdGKJM5F+PiK5XrGmWjKpdKm19shE3yg=; b=mS8khLyYAiuXE9Y0eSbOkTM1waA7XaNO+myuCtrysHrXCjJL4C0u1NHnQ+/H9yUVuW kfxrJr4f5NFUFlzzQ+RnkPOQRShGkCmaULUNj4XNjW6t3qrelyX4ZSQMzAmMdC1bOY6e 7iyMYcUfTR/MFIAYgFFKepwtU99B6ZyoaznW+qbDZOBizYHtbpKSp02xtHmpFqeaCVK6 jE0+W9R+OYV83gtpaaRp72Gti1yuJodhWD98pc6oUliMU5qfH93JeIh0tNkqDBdeZNno BPQOsgujb8cbv2smeDQeRc/Qni2XFWfpPmAoTrjBjFTSLIfSHhZzks+mtC56Eai8eFmZ hJ2g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=MVLvZFVN; 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=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k9-20020a17090646c900b0078e27f2fbe3si3483489ejs.115.2022.11.18.16.46.30; Fri, 18 Nov 2022 16:46:54 -0800 (PST) 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=pass header.i=@kernel.org header.s=k20201202 header.b=MVLvZFVN; 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=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238025AbiKSApz (ORCPT <rfc822;kkmonlee@gmail.com> + 99 others); Fri, 18 Nov 2022 19:45:55 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54814 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238016AbiKSApZ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 18 Nov 2022 19:45:25 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A53C8D9B88; Fri, 18 Nov 2022 15:47:28 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 50AA7B825A6; Fri, 18 Nov 2022 23:47:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 17700C433C1; Fri, 18 Nov 2022 23:47:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1668815246; bh=XihwsWyiavzhVTmCCaUHQYkuUf+CaFnN21aKdYIXar8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=MVLvZFVNR7rHcvbYuWk2IjlI/cpfz1iiyjTD0+2eaw6EHhqbW6MdRkCnCYdCtKlwS dwCkKmNYalqrgmln9eonH8/BAU3fDNvfT7z0Hzt7py4JSqrh3bnHrpgLo6reswibnH tISCTpcVwkKUKyWshudzZ9dvv47D0uD05qR0hKGXjwIJ1nx55LC2mNVcf6nqTvhMyL ECD5cpnLk3glAjZt7sHjCLg7FR8w75cwLAorze4WLNPKh+y4uiTdB1jflVllU5x2o4 npYEn0hF/B5bYLa0o14PWysnyhBBy0aBf+yfae3usfPKN9AoLdfTGdd2nMbDDkws0y 3D4Wd/spUdOFQ== Date: Fri, 18 Nov 2022 17:47:13 -0600 From: "Gustavo A. R. Silva" <gustavoars@kernel.org> To: "Martin K. Petersen" <martin.petersen@oracle.com>, "James E.J. Bottomley" <jejb@linux.ibm.com>, GR-QLogic-Storage-Upstream@marvell.com, Nilesh Javali <njavali@marvell.com> Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, "Gustavo A. R. Silva" <gustavoars@kernel.org>, linux-hardening@vger.kernel.org Subject: [PATCH 1/2][next] scsi: qla2xxx: Replace one-element array with DECLARE_FLEX_ARRAY() helper Message-ID: <faa40e6b31ecc9387ad1644bb1957aa53d7c682f.1668814746.git.gustavoars@kernel.org> References: <cover.1668814746.git.gustavoars@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <cover.1668814746.git.gustavoars@kernel.org> X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, 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?1749883357700329413?= X-GMAIL-MSGID: =?utf-8?q?1749883357700329413?= |
Series |
scsi: qla2xxx: Replace one-element array with flexible-array member
|
|
Commit Message
Gustavo A. R. Silva
Nov. 18, 2022, 11:47 p.m. UTC
One-element arrays as fake flex arrays are deprecated and we are moving
towards adopting C99 flexible-array members, instead. So, replace
one-element array declaration in struct ct_sns_gpnft_rsp, which is
ultimately being used inside a union:
drivers/scsi/qla2xxx/qla_def.h:
3240 struct ct_sns_gpnft_pkt {
3241 union {
3242 struct ct_sns_req req;
3243 struct ct_sns_gpnft_rsp rsp;
3244 } p;
3245 };
Important to mention is that doing a build before/after this patch results
in no binary differences.
This help us make progress towards globally enabling
-fstrict-flex-arrays=3 [1].
Link: https://github.com/KSPP/linux/issues/245
Link: https://github.com/KSPP/linux/issues/193
Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/scsi/qla2xxx/qla_def.h | 4 ++--
drivers/scsi/qla2xxx/qla_gs.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
Comments
On Fri, Nov 18, 2022 at 05:47:13PM -0600, Gustavo A. R. Silva wrote: > One-element arrays as fake flex arrays are deprecated and we are moving > towards adopting C99 flexible-array members, instead. So, replace > one-element array declaration in struct ct_sns_gpnft_rsp, which is > ultimately being used inside a union: > > drivers/scsi/qla2xxx/qla_def.h: > 3240 struct ct_sns_gpnft_pkt { > 3241 union { > 3242 struct ct_sns_req req; > 3243 struct ct_sns_gpnft_rsp rsp; > 3244 } p; > 3245 }; > > Important to mention is that doing a build before/after this patch results > in no binary differences. > > This help us make progress towards globally enabling > -fstrict-flex-arrays=3 [1]. > > Link: https://github.com/KSPP/linux/issues/245 > Link: https://github.com/KSPP/linux/issues/193 > Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1] > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org>
Le 19/11/2022 à 00:47, Gustavo A. R. Silva a écrit : > One-element arrays as fake flex arrays are deprecated and we are moving > towards adopting C99 flexible-array members, instead. So, replace > one-element array declaration in struct ct_sns_gpnft_rsp, which is > ultimately being used inside a union: > > drivers/scsi/qla2xxx/qla_def.h: > 3240 struct ct_sns_gpnft_pkt { > 3241 union { > 3242 struct ct_sns_req req; > 3243 struct ct_sns_gpnft_rsp rsp; > 3244 } p; > 3245 }; > > Important to mention is that doing a build before/after this patch results > in no binary differences. Hi, even with the: > rspsz = sizeof(struct ct_sns_gpnft_rsp) + > - ((vha->hw->max_fibre_devices - 1) * > + (vha->hw->max_fibre_devices * > sizeof(struct ct_sns_gpn_ft_data)); change ? CJ > > This help us make progress towards globally enabling > -fstrict-flex-arrays=3 [1]. > > Link: https://github.com/KSPP/linux/issues/245 > Link: https://github.com/KSPP/linux/issues/193 > Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1] > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/scsi/qla2xxx/qla_def.h | 4 ++-- > drivers/scsi/qla2xxx/qla_gs.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h > index a26a373be9da..1eea977ef426 100644 > --- a/drivers/scsi/qla2xxx/qla_def.h > +++ b/drivers/scsi/qla2xxx/qla_def.h > @@ -3151,12 +3151,12 @@ struct ct_sns_gpnft_rsp { > uint8_t vendor_unique; > }; > /* Assume the largest number of targets for the union */ > - struct ct_sns_gpn_ft_data { > + DECLARE_FLEX_ARRAY(struct ct_sns_gpn_ft_data { > u8 control_byte; > u8 port_id[3]; > u32 reserved; > u8 port_name[8]; > - } entries[1]; > + }, entries); > }; > > /* CT command response */ > diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c > index 64ab070b8716..69d3bc795f90 100644 > --- a/drivers/scsi/qla2xxx/qla_gs.c > +++ b/drivers/scsi/qla2xxx/qla_gs.c > @@ -4073,7 +4073,7 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type, srb_t *sp) > sp->u.iocb_cmd.u.ctarg.req_size = GPN_FT_REQ_SIZE; > > rspsz = sizeof(struct ct_sns_gpnft_rsp) + > - ((vha->hw->max_fibre_devices - 1) * > + (vha->hw->max_fibre_devices * > sizeof(struct ct_sns_gpn_ft_data)); > > sp->u.iocb_cmd.u.ctarg.rsp = dma_alloc_coherent(&vha->hw->pdev->dev,
On Sat, Nov 19, 2022 at 09:44:02AM +0100, Christophe JAILLET wrote: > Le 19/11/2022 à 00:47, Gustavo A. R. Silva a écrit : > > One-element arrays as fake flex arrays are deprecated and we are moving > > towards adopting C99 flexible-array members, instead. So, replace > > one-element array declaration in struct ct_sns_gpnft_rsp, which is > > ultimately being used inside a union: > > > > drivers/scsi/qla2xxx/qla_def.h: > > 3240 struct ct_sns_gpnft_pkt { > > 3241 union { > > 3242 struct ct_sns_req req; > > 3243 struct ct_sns_gpnft_rsp rsp; > > 3244 } p; > > 3245 }; > > > > Important to mention is that doing a build before/after this patch results > > in no binary differences. > > Hi, > > even with the: > > > rspsz = sizeof(struct ct_sns_gpnft_rsp) + > > - ((vha->hw->max_fibre_devices - 1) * > > + (vha->hw->max_fibre_devices * > > sizeof(struct ct_sns_gpn_ft_data)); > > change ? Yep; that change compensates for the removal of the 1 in the declaration of entries[]. The above piece of code is a common idiom to calculate the size for an allocation when a one-element array is involved. In the original code (vha->hw->max_fibre_devices - 1) compensates for the _extra_ size of one element of type struct ct_sns_gpn_ft_data in sizeof(struct ct_sns_gpnft_rsp). -- Gustavo > > CJ > > > > > This help us make progress towards globally enabling > > -fstrict-flex-arrays=3 [1]. > > > > Link: https://github.com/KSPP/linux/issues/245 > > Link: https://github.com/KSPP/linux/issues/193 > > Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1] > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > --- > > drivers/scsi/qla2xxx/qla_def.h | 4 ++-- > > drivers/scsi/qla2xxx/qla_gs.c | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h > > index a26a373be9da..1eea977ef426 100644 > > --- a/drivers/scsi/qla2xxx/qla_def.h > > +++ b/drivers/scsi/qla2xxx/qla_def.h > > @@ -3151,12 +3151,12 @@ struct ct_sns_gpnft_rsp { > > uint8_t vendor_unique; > > }; > > /* Assume the largest number of targets for the union */ > > - struct ct_sns_gpn_ft_data { > > + DECLARE_FLEX_ARRAY(struct ct_sns_gpn_ft_data { > > u8 control_byte; > > u8 port_id[3]; > > u32 reserved; > > u8 port_name[8]; > > - } entries[1]; > > + }, entries); > > }; > > /* CT command response */ > > diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c > > index 64ab070b8716..69d3bc795f90 100644 > > --- a/drivers/scsi/qla2xxx/qla_gs.c > > +++ b/drivers/scsi/qla2xxx/qla_gs.c > > @@ -4073,7 +4073,7 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type, srb_t *sp) > > sp->u.iocb_cmd.u.ctarg.req_size = GPN_FT_REQ_SIZE; > > rspsz = sizeof(struct ct_sns_gpnft_rsp) + > > - ((vha->hw->max_fibre_devices - 1) * > > + (vha->hw->max_fibre_devices * > > sizeof(struct ct_sns_gpn_ft_data)); > > sp->u.iocb_cmd.u.ctarg.rsp = dma_alloc_coherent(&vha->hw->pdev->dev, >
Le 19/11/2022 à 09:56, Gustavo A. R. Silva a écrit : > On Sat, Nov 19, 2022 at 09:44:02AM +0100, Christophe JAILLET wrote: >> Le 19/11/2022 à 00:47, Gustavo A. R. Silva a écrit : >>> One-element arrays as fake flex arrays are deprecated and we are moving >>> towards adopting C99 flexible-array members, instead. So, replace >>> one-element array declaration in struct ct_sns_gpnft_rsp, which is >>> ultimately being used inside a union: >>> >>> drivers/scsi/qla2xxx/qla_def.h: >>> 3240 struct ct_sns_gpnft_pkt { >>> 3241 union { >>> 3242 struct ct_sns_req req; >>> 3243 struct ct_sns_gpnft_rsp rsp; >>> 3244 } p; >>> 3245 }; >>> >>> Important to mention is that doing a build before/after this patch results >>> in no binary differences. >> >> Hi, >> >> even with the: >> >>> rspsz = sizeof(struct ct_sns_gpnft_rsp) + >>> - ((vha->hw->max_fibre_devices - 1) * >>> + (vha->hw->max_fibre_devices * >>> sizeof(struct ct_sns_gpn_ft_data)); >> >> change ? > > Yep; that change compensates for the removal of the 1 in the declaration > of entries[]. > > The above piece of code is a common idiom to calculate the size for an > allocation when a one-element array is involved. In the original code > (vha->hw->max_fibre_devices - 1) compensates for the _extra_ size of one > element of type struct ct_sns_gpn_ft_data in sizeof(struct ct_sns_gpnft_rsp). > Yes, I do agree, that the code is equivalent. I was surprised that a compiler was smart enough to generate the same binary code. With gcc 11.3.0 (x86_64), I do get some differences when I do: make drivers/scsi/qla2xxx/qla_gs.o objdump -D drivers/scsi/qla2xxx/qla_gs.o > before.asm patch -p1 < patch.diff make drivers/scsi/qla2xxx/qla_gs.o objdump -D drivers/scsi/qla2xxx/qla_gs.o > after.asm diff -u before.asm after.asm Mostly some slight ordering of instruction changes, but the binary are not the same. CJ > -- > Gustavo > >> >> CJ >> >>> >>> This help us make progress towards globally enabling >>> -fstrict-flex-arrays=3 [1]. >>> >>> Link: https://github.com/KSPP/linux/issues/245 >>> Link: https://github.com/KSPP/linux/issues/193 >>> Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1] >>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> >>> --- >>> drivers/scsi/qla2xxx/qla_def.h | 4 ++-- >>> drivers/scsi/qla2xxx/qla_gs.c | 2 +- >>> 2 files changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h >>> index a26a373be9da..1eea977ef426 100644 >>> --- a/drivers/scsi/qla2xxx/qla_def.h >>> +++ b/drivers/scsi/qla2xxx/qla_def.h >>> @@ -3151,12 +3151,12 @@ struct ct_sns_gpnft_rsp { >>> uint8_t vendor_unique; >>> }; >>> /* Assume the largest number of targets for the union */ >>> - struct ct_sns_gpn_ft_data { >>> + DECLARE_FLEX_ARRAY(struct ct_sns_gpn_ft_data { >>> u8 control_byte; >>> u8 port_id[3]; >>> u32 reserved; >>> u8 port_name[8]; >>> - } entries[1]; >>> + }, entries); >>> }; >>> /* CT command response */ >>> diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c >>> index 64ab070b8716..69d3bc795f90 100644 >>> --- a/drivers/scsi/qla2xxx/qla_gs.c >>> +++ b/drivers/scsi/qla2xxx/qla_gs.c >>> @@ -4073,7 +4073,7 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type, srb_t *sp) >>> sp->u.iocb_cmd.u.ctarg.req_size = GPN_FT_REQ_SIZE; >>> rspsz = sizeof(struct ct_sns_gpnft_rsp) + >>> - ((vha->hw->max_fibre_devices - 1) * >>> + (vha->hw->max_fibre_devices * >>> sizeof(struct ct_sns_gpn_ft_data)); >>> sp->u.iocb_cmd.u.ctarg.rsp = dma_alloc_coherent(&vha->hw->pdev->dev, >> >
> Yes, I do agree, that the code is equivalent. I was surprised that a > compiler was smart enough to generate the same binary code. We discard the tiny changes that don't affect the logic or the control flow of the program. -- Gustavo
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index a26a373be9da..1eea977ef426 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -3151,12 +3151,12 @@ struct ct_sns_gpnft_rsp { uint8_t vendor_unique; }; /* Assume the largest number of targets for the union */ - struct ct_sns_gpn_ft_data { + DECLARE_FLEX_ARRAY(struct ct_sns_gpn_ft_data { u8 control_byte; u8 port_id[3]; u32 reserved; u8 port_name[8]; - } entries[1]; + }, entries); }; /* CT command response */ diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index 64ab070b8716..69d3bc795f90 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -4073,7 +4073,7 @@ int qla24xx_async_gpnft(scsi_qla_host_t *vha, u8 fc4_type, srb_t *sp) sp->u.iocb_cmd.u.ctarg.req_size = GPN_FT_REQ_SIZE; rspsz = sizeof(struct ct_sns_gpnft_rsp) + - ((vha->hw->max_fibre_devices - 1) * + (vha->hw->max_fibre_devices * sizeof(struct ct_sns_gpn_ft_data)); sp->u.iocb_cmd.u.ctarg.rsp = dma_alloc_coherent(&vha->hw->pdev->dev,