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

From: Taylor Blau <me@ttaylorr.com>
Date: Jan 11, 2022, 19:06 UTC
Message-ID: <Yd3VKjvb7AR0/wI9@nand.local>
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.

Show 11 quoted lines

> 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