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

[Michael J Gruber <git@grubix.eu>]

Junio C Hamano venit, vidit, dixit 2022-05-25 00:34:40:
> Æ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. 

An integer can hold 438 perfectly, without any help by carefully
designed code.

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

A global variable cannot hold a meaningful pointer to a local variable,
unless the carefully designed code helps. So that "analogy" rather
highlights the essential difference, unless you think about a pointer
as "just some number" rather than "something that can be dereferenced".

[read global as outer scope, local as inner scope for simplicity]

Common practice is not necessarily good practice. In a traditional C
mindset, everything around pointers and memory management is doomed to
boom unless the code is designed carefully and "you know what you are
doing". I'm not indicating that you do not - on the contrary, you very
well do, as your detailed analysis of the code flow shows. At the same
time, it shows that we cannot be certain about that piece of code
without that detailed expert analysis.

C is not C++ nor rust, nor should it be; but the warnings and errors in
newer standards typically try to avoid those pitfalls by making sure
that, e.g., pointers do not go stale for "obvious reasons". They might
missflag cases where this is preempted for non-obvious reasons, but forcing
you to be explicit (more obvious) in your code is a good thing,
especially for maintainability of the code base.

Pointing from outer scope to memory in an inner scope should be a no-go;
that's what this error is about. Unsetting that pointer (by setting the
pointer to NULL) right before the inner scope ends is exactly the
right solution. If this "breaks" the code, the code is broken already.

Ironically, my original one line patch seems to work here, as your
detailed analysis shows. Truth in advertising: I arrived at that patch
after a considerably less detailed analysis ;)

Michael