From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] http API: fix dangling pointer issue noted by GCC 12.0
Date: Wed, 26 Jan 2022 22:30:40 +0100 [thread overview]
Message-ID: <patch-1.1-1cec367e805-20220126T212921Z-avarab@gmail.com> (raw)
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).
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".
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:
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");
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.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
A look at GCC's release history suggests that GCC 12.0 will be
released sometime in the next 3-6 months:
https://gcc.gnu.org/releases.html
By addressing this new -Wall warning early we can get ahead of the
annoying compilation error we'll run into with DEVELOPER=1 once it's
released.
http-walker.c | 4 ----
http.c | 8 +-------
http.h | 1 -
3 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/http-walker.c b/http-walker.c
index 910fae539b8..5cc369dea85 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -225,13 +225,9 @@ 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;
}
diff --git a/http.c b/http.c
index 229da4d1488..4507c9ac9c7 100644
--- a/http.c
+++ b/http.c
@@ -197,9 +197,6 @@ 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;
@@ -1204,7 +1201,6 @@ 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);
@@ -1327,10 +1323,8 @@ 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();
if (slot->in_use) {
diff --git a/http.h b/http.h
index df1590e53a4..81418d5fd8b 100644
--- a/http.h
+++ b/http.h
@@ -24,7 +24,6 @@ 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);
--
2.35.0.890.gd7e422415d9
next reply other threads:[~2022-01-26 21:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-26 21:30 Ævar Arnfjörð Bjarmason [this message]
2022-01-26 21:59 ` [PATCH] http API: fix dangling pointer issue noted by GCC 12.0 Taylor Blau
2022-01-27 0:50 ` Junio C Hamano
2022-01-27 0:57 ` Junio C Hamano
2022-01-27 3:45 ` Ævar Arnfjörð Bjarmason
2022-01-27 18:23 ` Junio C Hamano
2022-02-25 9:09 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2022-02-25 22:58 ` Junio C Hamano
2022-02-26 18:01 ` Taylor Blau
2022-03-25 14:34 ` [PATCH v3] " Ævar Arnfjörð Bjarmason
2022-03-25 18:11 ` Taylor Blau
2022-03-26 0:13 ` Junio C Hamano
2022-04-14 15:27 ` Ævar Arnfjörð Bjarmason
2022-04-14 17:04 ` Junio C Hamano
2022-04-15 13:30 ` Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=patch-1.1-1cec367e805-20220126T212921Z-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.