Re: [PATCH] http.c: clear the 'finished' member once we are done with it

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

On Tue, May 24 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> It doesn't mean that GCC has additionally proved that we'll later used
>> it in a way that will have a meaningful impact on the behavior of our
>> program, or even that it's tried to do that. See an excerpt from the GCC
>> code (a comment) in [1].
>
> But that means the warning just as irrelevant as "you stored 438 to
> this integer variable".  Sure, there may be cases where that integer
> variable should not exceed 400 and if the compiler can tell us that,
> that would be a valuable help to developers.  But "you stored an
> address of an object that can go out of scope in another object
> whose lifetime lasts beyond the scope" alone is not, without "and
> the caller that passed the latter object later dereferences that
> address here".  We certainly shouldn't -Werror on such a warning
> and bend our code because of it.

I think it says something that 1) we had exactly one of these in our
codebase 2) as we've discussed the pointer isn't actually *needed*
outside the scope of the function, it's just left-over.

Now, if it were used, e.g. let's say we had some code that took the
struct and inspected its members we'd likely have a segfault here, or
worse it would "work", but only on the platforms we'd test at first.

Which isn't the case with a leftover "int finished" holding a 438.

The point of this warning, like so many others, is to ask "hey, do you
really need to be running around with this particular pair of
scissors?".