On Fri, Sep 10 2021, Johannes Schindelin wrote:

Hi Ævar,

the commit title is misleading: it suggests that there is a bug that needs
to be fixed.

I picked "confusing" because this doesn't impact the end-state of the
program, it's just confusing to do:

    x = -1

Followed by:

    x = 0

Which the reader might wonder about, only to find that the initial
assignment wasn't needed or used for anything.

The idea of the patch, however, is to avoid redundant code, and if
described that way, the patch is a lot better for it.

On Thu, 9 Sep 2021, Ævar Arnfjörð Bjarmason wrote:

quoted

In be5d88e1128 (test-tool run-command: learn to run (parts of) the
testsuite, 2019-10-04) an init pattern was added that would use
TESTSUITE_INIT, but then promptly memset() everything back to 0. We'd
then set the "dup" on the two string lists. Our setting of "next" to
"-1" thus did nothing, we'd reset it to "0" before using it.

Let's just use the init macro for the STRING_LIST members, we can then
remove the already redundant memset().

Note that while we compile this code, there's no in-tree user for the
"testsuite" target being modified here anymore, see the discussion at
and around [ref][1].

1. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2109091323150.59@tvgsbejvaqbjf.bet/ (local)

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---

This patch is the immediate reason for why I submitted
https://lore.kernel.org/git/patch-1.1-d1e464da0a9-20210906T002938Z-avarab@gmail.com/ (local),
since Johannes would prefer to keep it let's fix this init pattern.

The diff does too many things, some of which are your purely personal
preferences and do not actually need to be changed. This is a much more
to-the-point diff:

We've been slowly converting everything to designated initializers. It
seems to make sense to just do that if the line is being touched anyway.

For instance my d385784f89b (fsck.h: use designed initializers for
FSCK_OPTIONS_{DEFAULT,STRICT}, 2021-03-28) was part of a series whose
initial version just changed one field at a time as you're doing below,
but during early review I was asked just to use designated initializers
already.

But yes, it is strictly unrelated. It's a judgement call when to do
cleanups on lines you touch while you're at it, v.s. turning one patch
into a potential series of really tiny changes.

Then there's the change of "struct foo x, y" to "struct foo x;\nstruct
foo y;\n" above. I changed that because that tends to be the usual
style, it also preempts feedback / reviewers having to look at the code
out-of-bounds to see what the struct looks like in the default diff
-U<n>.

So, in advance addressing another type of question, or needing to
describe the context, as you brought up in
https://lore.kernel.org/git/nycvar.QRO.7.76.6.2109091222260.59@tvgsbejvaqbjf.bet/ (local)
:)