[PATCH v2] http API: fix dangling pointer issue noted by GCC 12.0

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

The pre-release GCC 12.0 development branch has a new warning about
dangling pointers in -Wall:

    http.c: In function ‘run_active_slot’:
    http.c:1332:24: error: storing the address of local variable ‘finished’ in ‘*slot.finished’ [-Werror=dangling-pointer=]
     1332 |         slot->finished = &finished;
          |         ~~~~~~~~~~~~~~~^~~~~~~~~~~
    http.c:1330:13: note: ‘finished’ declared here
     1330 |         int finished = 0;
          |             ^~~~~~~~

This is on a locally built "gcc (GCC) 12.0.1 20220120 (experimental)",
built from gcc.git's 8bc700f4c3f (Enhance vec_pack_trunc for integral
mode mask., 2022-01-17).

The GCC warning is specifically about pointers that survive the exit
of the function. See a comment added to
"pass_waccess::use_after_inval_p" in the GCC commit that added the
warning, or:

    /* The use is one of a dangling pointer if a clobber of the variable
      [the pointer points to] has not been found before the function exit
      point.  */
    [...]

There's a few possible ways to fix this, but the simplest is to assign
NULL to "slot->finished" at the end of run_active_slot(), it's the
only caller that ever assigns non-NULL to it. It was suggested[2] to
guard that with "if (slot->finished == &finished)", but that'll still
trigger the warning.

1. https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9d6a0f388eb048f8d87f47af78f07b5ce513bfe6
2. https://lore.kernel.org/git/xmqq8rv2nggn.fsf@gitster.g/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

A much simpler fix for a warning new in the GCC v12 pre-release.

Range-diff against v1:
1:  1cec367e805 ! 1:  777838267a5 http API: fix dangling pointer issue noted by GCC 12.0
    @@ Commit message
         built from gcc.git's 8bc700f4c3f (Enhance vec_pack_trunc for integral
         mode mask., 2022-01-17).
     
    -    To fix this I first simply made the member "int finished",
    -    i.e. removing the pointer indirection. It turns out that nothing cared
    -    about the state of it being a NULL pointer v.s. "*ptr == 0".
    +    The GCC warning is specifically about pointers that survive the exit
    +    of the function. See a comment added to
    +    "pass_waccess::use_after_inval_p" in the GCC commit that added the
    +    warning, or:
     
    -    But we can instead amend the code added in baa7b67d091 (HTTP slot
    -    reuse fixes, 2006-03-10) to get rid of "int *finished" entirely. I
    -    instrumented the code to add this after every use of slot->finished or
    -    slot->in_use:
    +        /* The use is one of a dangling pointer if a clobber of the variable
    +          [the pointer points to] has not been found before the function exit
    +          point.  */
    +        [...]
     
    -        if (slot->finished && slot->in_use == *slot->finished) BUG("in-use = %d and finished = %d disconnect", slot->in_use, *slot->finished);
    -        if (!slot->finished && !slot->in_use) BUG("have !in-use and no finished pointer");
    +    There's a few possible ways to fix this, but the simplest is to assign
    +    NULL to "slot->finished" at the end of run_active_slot(), it's the
    +    only caller that ever assigns non-NULL to it. It was suggested[2] to
    +    guard that with "if (slot->finished == &finished)", but that'll still
    +    trigger the warning.
     
    -    Which never fires, but we would get occurrences of:
    -
    -        if (!slot->finished && slot->in_use) BUG("have in-use and no finished pointer");
    -
    -    I.e. we can simply drop the field and rely on "slot->in_use" in cases
    -    where we used "finished" before. The two fields were mirror images of
    -    each other, and the tri-state nature of "finished" wasn't something we
    -    relied upon.
    +    1. https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9d6a0f388eb048f8d87f47af78f07b5ce513bfe6
    +    2. https://lore.kernel.org/git/xmqq8rv2nggn.fsf@gitster.g/
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    - ## http-walker.c ##
    -@@ http-walker.c: static void process_alternates_response(void *callback_data)
    - 					 alt_req->url->buf);
    - 			active_requests++;
    - 			slot->in_use = 1;
    --			if (slot->finished != NULL)
    --				(*slot->finished) = 0;
    - 			if (!start_active_slot(slot)) {
    - 				cdata->got_alternates = -1;
    - 				slot->in_use = 0;
    --				if (slot->finished != NULL)
    --					(*slot->finished) = 1;
    - 			}
    - 			return;
    - 		}
    -
      ## http.c ##
    -@@ http.c: static void finish_active_slot(struct active_request_slot *slot)
    - 	closedown_active_slot(slot);
    - 	curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
    - 
    --	if (slot->finished != NULL)
    --		(*slot->finished) = 1;
    --
    - 	/* Store slot results so they can be read after the slot is reused */
    - 	if (slot->results != NULL) {
    - 		slot->results->curl_result = slot->curl_result;
    -@@ http.c: struct active_request_slot *get_active_slot(void)
    - 	active_requests++;
    - 	slot->in_use = 1;
    - 	slot->results = NULL;
    --	slot->finished = NULL;
    - 	slot->callback_data = NULL;
    - 	slot->callback_func = NULL;
    - 	curl_easy_setopt(slot->curl, CURLOPT_COOKIEFILE, curl_cookie_file);
     @@ http.c: void run_active_slot(struct active_request_slot *slot)
    - 	fd_set excfds;
    - 	int max_fd;
    - 	struct timeval select_timeout;
    --	int finished = 0;
    - 
    --	slot->finished = &finished;
    --	while (!finished) {
    -+	while (slot->in_use) {
    - 		step_active_slots();
    + 			select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
    + 		}
    + 	}
    ++	slot->finished = NULL;
    + }
      
    - 		if (slot->in_use) {
    -
    - ## http.h ##
    -@@ http.h: struct active_request_slot {
    - 	int in_use;
    - 	CURLcode curl_result;
    - 	long http_code;
    --	int *finished;
    - 	struct slot_results *results;
    - 	void *callback_data;
    - 	void (*callback_func)(void *data);
    + static void release_active_slot(struct active_request_slot *slot)

 http.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/http.c b/http.c
index 229da4d1488..2f67fbb33cd 100644
--- a/http.c
+++ b/http.c
@@ -1367,6 +1367,7 @@ void run_active_slot(struct active_request_slot *slot)
 			select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
 		}
 	}
+	slot->finished = NULL;
 }
 
 static void release_active_slot(struct active_request_slot *slot)
-- 
2.35.1.1175.gf9e1b23ea35