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

René Scharfe [off-list ref] writes:

Am 12.01.22 um 12:58 schrieb Han-Wen Nienhuys:

quoted

On Fri, Dec 24, 2021 at 5:16 AM Junio C Hamano [off-list ref] wrote:

quoted

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".

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html says about
-Wuninitialized:

   "Note that there may be no warning about a variable that is used only
    to compute a value that itself is never used, because such
    computations may be deleted by data flow analysis before the
    warnings are printed."

And indeed, dest is not used above.  But even if we change the funtion
to use dest by returning it, GCC versions 9.1 and higher don't warn
about the use of the uninitialized buffer.  GCC 4.7.1 to 8.5 do warn,
though: https://godbolt.org/z/zYz9KvPdK

What I meant in the original discussion was a runtime checker that
notices a read from an uninitialized location (I've also written one
myself way before my Git time, which was how I noticed strcpy() on
old SunOS, in an attempt to optimize by loading a word at a time,
sometimes read one word too much beyond the end of a page).

But if a static analysis may catch itas a potential error, that is
another reason to worry about it, too.

The original code wanted to do

	char message[100] = { 0 };

	for (i = 0; i < sizeof(message) - 1; i++)
        	message[i] = something();

	use_NUL_terminated_string(message);

and it allowed to omit an assigmnent

	message[sizeof(message) - 1] = '\0';

after the loop.

As I already said, I do not care too deeply in a test-only helper
like this.

Thanks.