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;
From:   Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To:     Bart Van Assche <bvanassche@acm.org>
CC:     Daniel Wagner <dwagner@suse.de>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        Chaitanya Kulkarni <kch@nvidia.com>,
        Max Gurtovoy <mgurtovoy@nvidia.com>,
        Hannes Reinecke <hare@suse.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        James Smart <jsmart2021@gmail.com>
Subject: Re: [PATCH blktests v1 01/11] nvme/{003,004,005,013,046,049}: Group
 all variables declarations
Thread-Topic: [PATCH blktests v1 01/11] nvme/{003,004,005,013,046,049}: Group
 all variables declarations
Thread-Index: AQHZv79EpGj72imkKEOvomc2O06946/MIqMAgAERAgCAAIgcgIAA5zkA
Date:   Fri, 28 Jul 2023 05:06:34 +0000
Message-ID: <ky32xyzyii2c5fr7kknyx5z6fikvh674v4sw2s4yux52l4vu7w@4nqmrmyxkwmw>
References: <20230726124644.12619-1-dwagner@suse.de>
 <20230726124644.12619-2-dwagner@suse.de>
 <7e4f6c51-43f4-c039-07bc-6724748a1d3e@acm.org>
 <kya2ayzcgz7iemknkx6xuef4m37f2ms7rt3uxamtxr2w7zbwfc@onladguxa7bk>
 <c3bff0d9-957a-fbb7-a433-4cab457c62bf@acm.org>
In-Reply-To: <c3bff0d9-957a-fbb7-a433-4cab457c62bf@acm.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Hkop6Dg3XpFOggF07zGbmM4xPYIKZ1FGf99+Bfhf+Lf37kMzIg8Q7iF/6dXT?=
 =?us-ascii?Q?tHF9PWwwmUoAobit+m27oo9od8Cq63SPzYnLd9Q5rBOOyFqnI3ltyaIfOn1A?=
 =?us-ascii?Q?eWCP9w/pBBVsRqWGy+DbKNjFD4uJW2bol7HKm654aYxFWAhNFpZwbdYi01ab?=
 =?us-ascii?Q?A5u8R4aZjpNGwCfRipYf1Ocb4ZuY57Ck28iZccLxWHw6vFJ01Y0WWRKM6jQ+?=
 =?us-ascii?Q?JqCf9ZPGUgOxWaGVhUIuoQNbB+ba7iMOmEvEnel3IOePXNQP/G4CrIKpJwrg?=
 =?us-ascii?Q?A4fHpA3WtE8hx1wBnpkI/8UTPROPYv4m/yYxWD6J5oWAJFWv6mJ4TS+6mZmY?=
 =?us-ascii?Q?EwVx89VlE2yUR/eZdy3QEvfRf4PaJpDB3jPVQvmLBL1ytWq4YhPGG5HjtYvZ?=
 =?us-ascii?Q?OmWkF/n3D8g1ZhTLTzn6JZNxgAvogQ7LJGKwyWeNKcpq/lObncY+WNEde8jp?=
 =?us-ascii?Q?ec25oxzP5koTOHpqwgYBJ/x+pcfCuKbRG8QitBJA4Lu9tdcYMSoqMPFpcyba?=
 =?us-ascii?Q?sUnRJUsiyq15tLw5inRA4/XejOqAIsr/fOnnhRL/aL+ec9GYqetex8jxNeGj?=
 =?us-ascii?Q?2xgq6v1Vl9tTSC6iE0UW+yPyqv/FuBWcrEPR9YMEW7I9FW4Lpue+EZkVBgzv?=
 =?us-ascii?Q?cwdGrdVmM8fUWZpweywSMsYTFBLDC0GrQyRV9Oao9UPfHDFFuCsxk8Ut7rvu?=
 =?us-ascii?Q?tvlnlHnDrR2C4I3VvuY5uY27a0vW3xW15DMwSEYqDsCyCE+hZXvz8Zqmyk3P?=
 =?us-ascii?Q?zZsW9yhAOS72UgAiraJmL7ou8NuEUE3Ylt2kV8Wv3rTx5a0ytHy7PEScjE49?=
 =?us-ascii?Q?hDsUX8AZVvudfFBCQQh+Da287ogI6ZkBqRRj6hkbrh/VvMtwcAZlXE+2cDgL?=
 =?us-ascii?Q?jHofrqvbhj+J1Ic2Rq0nRwRCDZ7vUNevzDw2WUqriDkBc3dhFjs62gAaamaf?=
 =?us-ascii?Q?y9cL9/0x1dicjrj98zOgtm4SKqYUKzUouP7nvs2VvEbVc+H05wP0TzvMtWeW?=
 =?us-ascii?Q?dUd7O4DIRqKtsDU9ShQYIxBmSouaugie02Pmz0qy5tvYZUBhJGvM56Wk9Kw+?=
 =?us-ascii?Q?zbx4nflL4CHTvqIs2G/qlDklTKB45Y366RiyulEySUWxn+KPEGYSxKaqK5QX?=
 =?us-ascii?Q?PiFRenVQb7oqSPFr0+Crooj7liVapFFetYGjLgwqk41NpQVQOVuBQowLYU+l?=
 =?us-ascii?Q?OzN9XeRnPOuzbPxMUCY40R+io3L7XWVLOmEI7KNZgTw8/KQHeXsa2Fu70kYz?=
 =?us-ascii?Q?63k1X80HbvAyjVNVIiqiqV8pWpp9wVdX3xg+2U4mmiHbEigAAho+8Rjvl6Im?=
 =?us-ascii?Q?165PKtO9RMI2j6Fs0UubTLdF9y4YhB0vnNXdDYDNfEsvqAx/A+CkiznmVa6N?=
 =?us-ascii?Q?JrMlite7dFe5M62EHNoaRPk3BBxJzRShpHi2aR97rPJ4zuQi+QdcEmXXyVGy?=
 =?us-ascii?Q?zlUwYHoLHAqHQ+/yWx0gH1ddQA1zx/UliS7eblYjluYCNfqdeSxkbvRlh+mJ?=
 =?us-ascii?Q?mjyrzYZbVJNm9sa1fdCySsrDT92R2EmzuXTPa38ziv0k9amgUR/8Rp3B8OiJ?=
 =?us-ascii?Q?xk3lC6NjmAQdZhwkmeDv5qgC8Zc37GZfmGTFjZxihP0+hOqiu/ZBY2jjY3QW?=
 =?us-ascii?Q?MSDeixvpF3+KQKKlrEkAn9g=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <36F5A8830207EC4BA90091848C866EAA@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?yQbHx3VAvNreg8EXA5U8a4CxqCZBD0HoRAkBBemwtlpo9JoNYB0a69/PhYdj?=
 =?us-ascii?Q?kJ3E0yIZ1EDmoJwo0pS6Tpk12SyLp4fGtGi52/2WcgyEiJr4YP792zzIRI7G?=
 =?us-ascii?Q?mr/smpfbGShLD6g17RMz5NTymwhm8+zSX+y52/XTGBz+VYJs+UAgBQON5m8h?=
 =?us-ascii?Q?GAVtxDwaBypI6nVgnsI8nKDJPBbB05H/rAhXbBaOqLsE03lIXgoBzhlB6zdu?=
 =?us-ascii?Q?Iqzqw4YV7XqJHiOBmHuivtjLK24plLtzIF18nPHV2prh+MJDMPHOUIpDEjVx?=
 =?us-ascii?Q?OBonz62OUJwUyqjGGfWDdkPkmAZkMW0I5KhKlnQpA1exIPdtybpBgM2AMIQd?=
 =?us-ascii?Q?9SJI0raoBSeq6m7BB87+Qds8Sbf8kNbJp0izcsFxqSEP9ixwkJGjU1U5XXm3?=
 =?us-ascii?Q?ipp4WFOqDVmSuTE0idbPzukF10V2S2GOfEtgHANBrBZ7Ia2gapFxT1f3/MNg?=
 =?us-ascii?Q?Innt8dAgMh5kc8Woep5kmvA+ps+IoPSg6hC6nFU8/Z+3TouTA4qQWzz7QFXH?=
 =?us-ascii?Q?9GuUgxGtNevbwRL6DHDmzvHjphDKIKwXMIVOdqKNwPlPP4xxJG5AD5e29Wb0?=
 =?us-ascii?Q?1fwv0nQEPyShFkP425sBykNpjq5oFe+OjaNIgIgXj/djiqz5GhuIXJboPQCL?=
 =?us-ascii?Q?ldsw+rAzM44I3DiZBLEnvm27VN2nDw9wJWT3mSEcsSgnISzOOnKgzuQa1YsK?=
 =?us-ascii?Q?llCwwpvX+tJ712Z+0F/VUyJyEnASAH19TgN3Z8q9Uyv/lCRuXqcvQof2PfgZ?=
 =?us-ascii?Q?38U+mxhJoTTqOeOq6CzSYXQq4QYis5t1PVkaFNUimNAnCGBacsr8g9cNiP2N?=
 =?us-ascii?Q?5AZZsafN0YQgG+FsFgYe7PHaC0EVQhrggqdZKTVFvaYTN7qo6U/a0ppz8MsO?=
 =?us-ascii?Q?4Y72Ur2eACQWplySo8rHw02LUM6ndmN7HKGrB6GlCBQIgT2bz44WqcMlFNYY?=
 =?us-ascii?Q?twvPQWEv1eWXF9mwRc50AA=3D=3D?=
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM8PR04MB8037.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3c3cfd70-d984-4b12-ef75-08db8f2869e8
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Jul 2023 05:06:34.1247
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ZeRgU/OEiBzB9AL1HcnU17bqoKVNXolLVKT9B9sezyWCIqD72+jCjdCTGXpe4dJ2pROcHGgOl7MgxeVtRgs/bjT11q4gyA3CsJbsjFIPQXo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR04MB8119
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        SPF_HELO_PASS,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        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

On Jul 27, 2023 / 08:18, Bart Van Assche wrote:
> On 7/27/23 00:11, Daniel Wagner wrote:
> > On Wed, Jul 26, 2023 at 07:54:24AM -0700, Bart Van Assche wrote:
> > > On 7/26/23 05:46, Daniel Wagner wrote:
> > > > Group all variable declarations together at the beginning of the
> > > > function.
> > >=20
> > > An explanation of why this change has been proposed is missing from t=
he
> > > patch description.
> >=20
> > Sure, I'll add one. The coding style to declare all local variables at =
the
> > beginning of the function.
>=20
> Isn't declaring local variables just before their first use a better styl=
e?

IMO both styles have pros and cons. Declarations at "beginning of functions=
"
helps to understand what the function uses as its local data (pros), but th=
e
declaration and the usage are separated and makes it difficult to understan=
d
(cons). Declarations at "just before first use" have the opposite pros and =
cons.
This style is easier to read especially when a function is rather long.

In the past, I preferred declarations at the beginning functions and reques=
ted
it in my review comments [1], but I learned that this guide is not so widel=
y
applied: xfstests scripts, or even blktests 'check' scripts have declaratio=
ns in
the middle of the functions. So I think both styles are okay at this moment=
.

  [1] https://github.com/osandov/blktests/pull/99

More importantly, this discussion maybe going towards "too strict" guidelin=
es,
which will discourage contributions. Similar topic is [[ ]] vs [ ]. Once I =
was
requesting strictly to use [[ ]], but it did not seem productive. Now I no
longer request to replace [ ] with [[ ]]. In same manner, I suggest not to =
be
strict on the local variable declaration position either.

As for this patch, it is not required to follow guidelines. Does it make
Daniel's refactoring work easier? If so, I guess it will be valuable.