Re: [PATCH] Fix maybe-uninitialized warnings found by gcc 9 -flto
- From
René Scharfe <l.s.r@web.de>
- Date
- Sep 5, 2019, 19:39 UTC
- Message-ID
- <8d90d803-8a4b-2656-550d-ccee5156b643@web.de>
- In-Reply-To
- <xmqqr24upmtm.fsf@gitster-ct.c.googlers.com>
Am 05.09.19 um 21:25 schrieb Junio C Hamano:
Show 28 quoted lines
> René Scharfe <l.s.r@web.de> writes:
>
>> Am 05.09.19 um 19:53 schrieb Jeff King:
>>>>> int cmd__read_cache(int argc, const char **argv)
>>>>> {
>>>>> - int i, cnt = 1, namelen;
>>>>> + int i, cnt = 1, namelen = 0;
>>>
>>> I actually saw this one the other day, because it triggered for me when
>>> compiling with SANITIZE=address. AFAICT it's a false positive. "name" is
>>> always NULL unless skip_prefix() returns true, in which case we always
>>> set "namelen". And we only look at "namelen" if "name" is non-NULL.
>>>
>>> This one doesn't even require LTO, because skip_prefix() is an inline
>>> function. I'm not sure why the compiler gets confused here.
>>
>> Yes, that's curious.
>>
>>> I don't mind
>>> initializing namelen to 0 to silence it, though (we already set name to
>>> NULL, so this would just match).
>>
>> Pushing the strlen() call into the loop and getting rid of namelen should
>> work as well -- and I'd be surprised if this had a measurable performance
>> impact.
>
> Yeah, we are making strlen() call on a constant "name" in a loop
> over argv[]. I do not think it matters in this case, either.The loop count is either 1 or argv[1] interpreted as a number, i.e. it could be very high. Its body consists of an index load and writing a number to a file, though -- a strlen() call on the name of that file should go unnoticed amid that activity. (I didn't measure it, though.)
René