Re: [PATCH v5 02/16] reftable: fix resource leak in block.c error path

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Jan 12 2022, Han-Wen Nienhuys wrote:

> On Fri, Dec 24, 2021 at 5:16 AM Junio C Hamano <gitster@pobox.com> wrote:
>> Once you
>> initialize at the declaration with "less meaningful" value (like
>> zero initialization), the tools won't be able to tell when the code
>> uses that variable "uninitialized" (because the assignment was
>> skipped by a bug), since it appears to always be initialied to them.
>
> Which tools are these? When I add
>
> static void test_memcpy(void)
> {
>  uint32_t dest;
>  char not_init[200];
>  int i;
>  memcpy(&dest, not_init, sizeof(dest));
>
>  for (i = 0 ; i < 10; i++)
>   not_init[i] = rand() % 255 + 1;
>  printf("%d", (int) strlen(not_init));
> }
>
> to the C code, it compiles cleanly if I do "make DEVELOPER=1".

Aside from what René said in his follow-up, I think what Junio's
pointing out here has to do with the use of this pattern in general, not
the specific code being discussed here.

I.e. if you get into the habit of needless initialization it may not
matter right now, but once the function grows an if/else branch, or
someone copies the pattern to such a function it may hide a logic error.

So it's not about analyzing the control specific flow here, but that
your upthread patch is separating a variable and its actual
internalization by ~20 lines.

So in the general case, by initializing it when it's declared it's more
likely that we'll introduce a logic error where it won't be initialized
at all.