Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()

From: Lars Schneider <larsxschneider@gmail.com>
Date: Aug 27, 2017, 20:04 UTC
Message-ID: <179AC8FB-5991-4200-9AAC-2F8D0914D5F9@gmail.com>
In-Reply-To: <CAN0heSraJFbbog7FKpAtmob9W6_5-AS1StZFVW6xUwMDWfMYgg@mail.gmail.com>

Show 17 quoted lines

> On 27 Aug 2017, at 21:09, Martin Ågren <martin.agren@gmail.com> wrote:
> 
> On 27 August 2017 at 20:21, Lars Schneider <larsxschneider@gmail.com> wrote:
>> 
>>> On 27 Aug 2017, at 09:37, Martin Ågren <martin.agren@gmail.com> wrote:
>>> 
>>> The static-ness was silently dropped in commit 70428d1a5 ("pkt-line: add
>>> packet_write_fmt_gently()", 2016-10-16). As a result, for each call to
>>> packet_write_fmt_1, we allocate and leak a buffer.
>> 
>> Oh :-( Thanks for detecting and fixing the leak.
>> 
>> How did you actually find it? Reading the code or did you use some
>> tool?
> 
> Valgrind found it for me. Most leaks it reports are "ok" since we're
> about to exit, it just takes more or less effort to realize that...

I am sorry if I am bugging you here, but I would really like to understand how you found it.

I did run all tests under valgrind using "make valgrind" and I found the following files with potential issues:

cat valgrind.out | perl -nE 'say /^==.+\((.+)\:.+\)$/' | sort | uniq -c
7102
   2 clone.c
  33 common-main.c
   6 connect.c
  64 git.c
   4 ls-remote.c
 126 run-command.c
  12 transport.c
   7 worktree.c

No mention of "pkt-line.c". Did you run Git with valgrind on one of your repositories to find it?

Thanks, Lars