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

Shinichiro Kawasaki shinichiro.kawasaki at wdc.com
Thu Jul 27 22:06:34 PDT 2023


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.



More information about the Linux-nvme mailing list