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

Re: [PATCH next v2] log_ref_setup: don't return stack-allocated array

From
Erick Mattos <erick.mattos@gmail.com>
Date
Jun 11, 2010, 18:54 UTC
Message-ID
<AANLkTikhgl2b_66POXPf1nJSlhwkY5PV1Qce3cA9yXOx@mail.gmail.com>
In-Reply-To
<20100611051236.GA3947@coredump.intra.peff.net>
Hi,
2010/6/11 Jeff King <peff@peff.net>:
Show 5 quoted lines
> No, Thomas is right. This invokes undefined behavior. We point the
> passed-in log_file pointer to the front of a character array with
> automatic duration. After log_ref_setup returns, we must never
> dereference that pointer again, but we do. So we need this patch or
> something like it.

You and Thomas are right on this subject. I don't know when and how I could see a malloc() in git_(v)snpath(). My fault.

Show 5 quoted lines
>> Then git_snpath() creates a char array in the heap with the right
>> content and changes the stack pointer logfile to it.  Then when we do
>
> No, it doesn't. git_snpath writes into the buffer you provide it, just
> like snprintf (hence the name).
The source of my failure.
Show 15 quoted lines
> We have some false positives in git, but you don't see them because
> t/valgrind/default.supp suppresses them. For example:
>
>  http://thread.gmane.org/gmane.comp.version-control.git/106335/focus=107302
>
> If you are using a binary package of valgrind, it probably ships with
> some system-specific suppressions, too. Right now valgrind on Debian
> unstable is next to useless because glibc has been upgraded to 2.11, but
> the suppressions haven't been updated. So you get false positives all
> over the place because of clever architecture-specific optimizations
> (e.g., I am seeing a lot of __strlen_sse2 problems, which are probably
> just the function over-reading its input data because processing big
> chunks is faster).
>
> -Peff
Thanks for the extended explanation about valgrind.
Regards to all
Previous: Jeff KingNext: Ævar Arnfjörð Bjarmason
Message 7 of 11 in “log_ref_setup: don't return stack-allocated array”
  1. log_ref_setup: don't return stack-allocated arrayThomas Rast, Jun 10, 2010
  2. log_ref_setup: don't return stack-allocated arrayThomas Rast, Jun 10, 2010
  3. Erick MattosJun 10, 2010
  4. Thomas RastJun 10, 2010
  5. Erick MattosJun 10, 2010
  6. Jeff KingJun 11, 2010
  7. Erick MattosJun 11, 2010
  8. Ævar Arnfjörð BjarmasonJun 10, 2010
  9. check_aliased_update: strcpy() instead of strcat() to copyThomas Rast, Jun 10, 2010
  10. Ævar Arnfjörð BjarmasonJun 10, 2010
  11. Jay SoffianJun 10, 2010

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.