Re: [PATCH 1/3] test-tool genzeros: initialize "zeros" to avoid SunCC warning

From: Taylor Blau <me@ttaylorr.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Han-Wen Nienhuys <hanwen@google.com>
Subject: Re: [PATCH 1/3] test-tool genzeros: initialize "zeros" to avoid SunCC warning
Date: Tue, 11 Jan 2022 14:06:18 -0500	[thread overview]
Message-ID: <Yd3VKjvb7AR0/wI9@nand.local> (raw)
In-Reply-To: <patch-1.3-4dadf7320ab-20220111T163908Z-avarab@gmail.com>

On Tue, Jan 11, 2022 at 05:40:21PM +0100, Ævar Arnfjörð Bjarmason wrote:
> It isn't important for optimization to have this be "static", so let's
> just initialize it and avoid this warning on Sun Studio 12.5:
>
>     "t/helper/test-genzeros.c", line 7: warning: const object should have initializer: zeros

I agree that whether or not this variable is declared statically does
not matter for our purposes, since we call cmd__genzeros() exactly once
per invocation of the test helper. So we're never paying the cost to
re-declare a variable on the stack since that stack frame only gets
created once.

And I don't care for the purposes of a reroll on such a trivial
question, but I do think that this patch leaves something out that was
included in the cover letter. Namely, that this is not the first such
warning of this kind from SunCC. Or in other words, that we have lots of
static const objects that we depend on being zero'd (and which we can
safely assume _are_ zerod, since they are declared statically), but that
each of them already generates a warning from SunCC.

Mentioning that would give readers an impression of why this was the
only spot touched instead of every static const variable.

But if you take that line of thinking further, I have a hard time
imagining that it's worth fixing small issues like this in piecemeal
when there are already many such warnings. We _could_ fix all of them at
once and amend Documentation/CodingGuidelines, but that seems like a lot
of work for an issue that does not seem to be causing much pain at all.

> diff --git a/t/helper/test-genzeros.c b/t/helper/test-genzeros.c
> index 8ca988d6216..5dc89eda0cb 100644
> --- a/t/helper/test-genzeros.c
> +++ b/t/helper/test-genzeros.c
> @@ -3,8 +3,7 @@
>
>  int cmd__genzeros(int argc, const char **argv)
>  {
> -	/* static, so that it is NUL-initialized */
> -	static const char zeros[256 * 1024];
> +	const char zeros[256 * 1024] = { 0 };

So this transformation is just fine, but I imagine that we probably
could have just as easily left it alone.

Thanks,
Taylor