git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Carlo Arenas <carenas@gmail.com>
Date
May 7, 2022, 19:11 UTC
Message-ID
<CAPUEspjm2_6Omk1_VanXZJnRREnLS08H4t9tbxx=dnoqA+P43g@mail.gmail.com>
In-Reply-To
<xmqq4k21gp6g.fsf@gitster.g>
On Sat, May 7, 2022 at 11:42 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 25 quoted lines
>
> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
>
> > On Fri, May 06, 2022 at 02:17:01PM -0700, Junio C Hamano wrote:
> >> diff --git a/http.c b/http.c
> >> index 229da4d148..85437b1980 100644
> >> --- a/http.c
> >> +++ b/http.c
> >> @@ -1367,6 +1367,9 @@ void run_active_slot(struct active_request_slot *slot)
> >>                      select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
> >>              }
> >>      }
> >> +
> >> +    if (slot->finished == &finished)
> >> +            slot->finished = NULL;
> >
> > I am not completely sure yet (since I looked at it long ago and got
> > sidetracked) but I think this might be optimized out (at least by gcc12)
> > since it is technically UB, which is why it never "fixed" the warning.
>
> UB meaning "undefined behaviour"?  Which part is?  Taking the
> address of an on-stack variable "finished"?
> Comparing it with a
> pointer that may or may not have been assigned/overwritten elsewhere
> in a structure?

it is not very intuitive, but using a pointer to a variable that is out of scope is UB, and in this case the value of slot->finished might point to an address that is not in our own stack (because it came from a different thread), hence undefined

Carlo
Previous: Junio C HamanoNext: Johannes Schindelin
Message 11 of 38 in “quell a few gcc warnings”
  1. 0/2 quell a few gcc warningsMichael J Gruber, May 6, 2022
  2. 1/2 dir.c: avoid gcc warningMichael J Gruber, May 6, 2022
  3. Junio C HamanoMay 6, 2022
  4. Taylor BlauMay 9, 2022
  5. Carlo Marcelo Arenas BelónMay 7, 2022
  6. 2/2 http.c: avoid gcc warningMichael J Gruber, May 6, 2022
  7. Junio C HamanoMay 6, 2022
  8. http.c: clear the 'finished' member once we are done with itJunio C Hamano, May 6, 2022
  9. Carlo Marcelo Arenas BelónMay 7, 2022
  10. Junio C HamanoMay 7, 2022
  11. Carlo ArenasMay 7, 2022
  12. Johannes SchindelinMay 23, 2022
  13. Junio C HamanoMay 23, 2022
  14. Junio C HamanoMay 23, 2022
  15. Johannes SchindelinMay 23, 2022
  16. Junio C HamanoMay 24, 2022
  17. Daniel StenbergMay 24, 2022
  18. Johannes SchindelinMay 24, 2022
  19. Junio C HamanoMay 24, 2022
  20. Daniel StenbergMay 26, 2022
  21. Johannes SchindelinMay 24, 2022
  22. Junio C HamanoMay 24, 2022
  23. Carlo Marcelo Arenas BelónMay 24, 2022
  24. Ævar Arnfjörð BjarmasonMay 24, 2022
  25. Junio C HamanoMay 24, 2022
  26. Michael J GruberMay 25, 2022
  27. Ævar Arnfjörð BjarmasonMay 25, 2022
  28. Junio C HamanoMay 24, 2022
  29. Junio C HamanoMay 24, 2022
  30. Carlo ArenasMay 25, 2022
  31. Ævar Arnfjörð BjarmasonMay 24, 2022
  32. Junio C HamanoMay 24, 2022
  33. Johannes SchindelinMay 25, 2022
  34. Junio C HamanoMay 25, 2022
  35. Carlo Marcelo Arenas BelónMay 6, 2022
  36. detect-compiler: make detection independent of localeMichael J Gruber, May 9, 2022
  37. Junio C HamanoMay 9, 2022
  38. rsbecker@nexbridge.comMay 9, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.