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

Hi,

2010/6/11 Jeff King [off-list ref]:

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.

quoted

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.

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