linux-kernel - Re: [PATCH blktests v1 01/11] nvme/{003,004,005,013,046,049}: Group all variables declarations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 28 Jul 2023 05:06:34 +0000
From:   Shinichiro Kawasaki <shinichiro.kawasaki@....com>
To:     Bart Van Assche <bvanassche@....org>
CC:     Daniel Wagner <dwagner@...e.de>,
        "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        Chaitanya Kulkarni <kch@...dia.com>,
        Max Gurtovoy <mgurtovoy@...dia.com>,
        Hannes Reinecke <hare@...e.de>,
        Sagi Grimberg <sagi@...mberg.me>,
        James Smart <jsmart2021@...il.com>
Subject: Re: [PATCH blktests v1 01/11] nvme/{003,004,005,013,046,049}: Group
 all variables declarations

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.
> > > 
> > > An explanation of why this change has been proposed is missing from the
> > > patch description.
> > 
> > Sure, I'll add one. The coding style to declare all local variables at the
> > beginning of the function.
> 
> Isn't declaring local variables just before their first use a better style?

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 the
declaration and the usage are separated and makes it difficult to understand
(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 requested
it in my review comments [1], but I learned that this guide is not so widely
applied: xfstests scripts, or even blktests 'check' scripts have declarations 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" guidelines,
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.