Re: [PATCH 02/12] load_subtree(): remove unnecessary conditional
Junio, I'm surprised that you have merged the `mh/notes-cleanup` branch into `next` already. Was that intentional? Aside from the fact that the topic has had very little cooking time, there's the issue of the assertion that you asked for. I have implemented the assertion in a new version of the branch that I pushed to GitHub but haven't yet submitted to the list. How would you like to proceed? Do you want me to submit a patch (adding the assertion) that applies on top of this branch? Michael
[PATCH] format-patch: use raw format for notes
If "--notes=..." is used with "git format-patch", the notes are prefixed with the ref's local name and indented, which looks odd and exposes the path of the ref. Extend the test that suppresses this behaviour so that it also catches this case, causing the notes to be included without additional processing. Signed-off-by: Sam Bobroff --- Notes (foo): Hi, I've noticed what appears to be a small cosmetic bug in git format-patch, as I've described in the commit message. I'm not sure if this patch is the right way to fix it (or perhaps it's not even a bug), but it should at least help to explain what I'm talking about. I've used "git format-patch --notes=foo" to prepare this email so that it is an example of the issue :-) Cheers, Sam. log-tree.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/log-tree.c b/log-tree.c index 410ab4f02..26bc21ad3 100644 --- a/log-tree.c +++ b/log-tree.c @@ -655,7 +655,8 @@ void show_log(struct rev_info *opt) int raw; struct strbuf notebuf = STRBUF_INIT; - raw = (opt->commit_format == CMIT_FMT_USERFORMAT); + raw = (opt->commit_format == CMIT_FMT_USERFORMAT) || + (opt->commit_format == CMIT_FMT_EMAIL); format_display_notes(&commit->object.oid, ¬ebuf, get_log_output_encoding(), raw); ctx.notes_message = notebuf.len -- 2.11.0
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
On 28 August 2017 at 01:23, Jeff King wrote: > On Sun, Aug 27, 2017 at 10:04:55PM +0200, Lars Schneider wrote: > >> 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 > > I'm not sure where valgrind.out comes from. The individual > test-results/*.out files may have valgrind output, but I don't think > they usually contain leak output. > > Doing "valgrind ./git-upload-pack . /dev/null" mentions > leaked memory but not the locations. Adding --leak-check=full shows that > most of it comes from format_packet(). > > And applying Martin's patch drops the "definitely lost" category down to > 0 bytes (there's still 550k in "still reachable", but those are in the > "exit will free them for us" category). > >> No mention of "pkt-line.c". Did you run Git with valgrind on one of >> your repositories to find it? > > I'm curious, too. I don't think the valgrind setup in our test suite is > great for finding leaks right now. Sorry for being brief. I've patched t/valgrind/valgrind.sh to say "--leak-check=yes". Then I run "./t --valgrind", simply because running the complete suite gives more reports than I could possibly process. Then I check the first few leaks, verify that they're "ok" and add them to a suppressions-list. Lather, rinse, repeat. A couple of very targeted and well-motivated suppressions in git.git could perhaps be motivated, but there are many many reported leaks. My suppressions-list is getting gross. I started with t and t0001 because I figure, once I have those basic suppressions in place (or leaks fixed), I can run some other more interesting tests. Of course, the concept of "this leak is ok" is a bit subjective. For example, we might do "return !!create_object(...);" (function name invented on the fly), which is a leak, and unreachable. But if this is only done once in builtin/foo.c and the object created is small, then this could be deemed "ok", since in practice this leak will never bring anyone over the cliff. If clean-ups in such code would not just be code churn, then I can of course adjust my definition of "ok" accordingly. This is not an attempt to find and fix a huge number of leaks, it's more to have a good reason to go through call-stacks, convince myself I know what the code wants to do and how it does it. Looking at only "unreachable" leaks seems like it should be an improvement for finding the interesting cases. I'll have less time for Git this week, but can try it out as time permits. Thanks for your feedback, both of you. Martin
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
On Sun, Aug 27, 2017 at 10:04:55PM +0200, Lars Schneider wrote: > 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 I'm not sure where valgrind.out comes from. The individual test-results/*.out files may have valgrind output, but I don't think they usually contain leak output. Doing "valgrind ./git-upload-pack . /dev/null" mentions leaked memory but not the locations. Adding --leak-check=full shows that most of it comes from format_packet(). And applying Martin's patch drops the "definitely lost" category down to 0 bytes (there's still 550k in "still reachable", but those are in the "exit will free them for us" category). > No mention of "pkt-line.c". Did you run Git with valgrind on one of > your repositories to find it? I'm curious, too. I don't think the valgrind setup in our test suite is great for finding leaks right now. -Peff
Loan Offer!
Loan Offer at 3%, Feel Free to REPLY back to us for more info.
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
> On 27 Aug 2017, at 21:09, Martin Ågren wrote:
>
> On 27 August 2017 at 20:21, Lars Schneider wrote:
>>
>>> On 27 Aug 2017, at 09:37, Martin Ågren 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
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
On Sun, Aug 27, 2017 at 09:09:15PM +0200, Martin Ågren wrote:
> On 27 August 2017 at 20:21, Lars Schneider wrote:
> >
> >> On 27 Aug 2017, at 09:37, Martin Ågren 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...
This is one more thing it would be nice to have as part of our
perf-testing framework. It will run two versions of Git across a battery
of tests and report on the runtime differences for each. It would be
great if it did the same for peak heap. The tricky thing is that you
sometimes need repositories that are exaggerated in size in one
dimension to notice the differences as significant. So I don't know if
we would need new tests for this, or if existing "this repo has a lot of
refs" tests would have caught this.
Another approach, of course, is to get valgrind (or asan) to a
zero-leaks-reported steady state, and then even small leak reports would
be worth investigating. I'm not sure how easy it is to get there, and
whether it's less work to actually plug free-on-exit leaks or somehow
suppress the reports. I think most free-on-exit false positives would
show up as "leaked but still reachable", whereas leaks like this that
can grow without bound mean would not be reachable (we throw away the
pointer to the memory at the end of the function).
-Peff
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
On 27 August 2017 at 20:21, Lars Schneider wrote:
>
>> On 27 Aug 2017, at 09:37, Martin Ågren 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...
Martin
RE: cat-file timing window on Cygwin
> -Original Message-
> From: Ramsay Jones
> Sent: Sunday, August 27, 2017 11:48 AM
> To: Adam Dinwoodie
> Cc: Jeff King; git@vger.kernel.org; Johannes Schindelin
> Subject: Re: cat-file timing window on Cygwin
>
>
>
> On 27/08/17 12:33, Adam Dinwoodie wrote:
> > On Sun, Aug 27, 2017 at 03:06:31AM +0100, Ramsay Jones wrote:
> >> On 26/08/17 22:11, Adam Dinwoodie wrote:
> >>> On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:
> Interesting. I find it a little hard to believe there's
> so obvious
> a bug as "fflush(NULL) flushes stdin", but well...that's
> what it seems like.
>
> If that's truly what it is, this is the minimal
> reproduction I came
> up
> with:
>
> -- >8 --
> #include
>
> int main(void)
> {
> char buf[256];
> while (fgets(buf, sizeof(buf), stdin)) {
> fprintf(stdout, "got: %s", buf);
> fflush(NULL);
> }
> return 0;
> }
> -- 8< --
>
> If this really is the bug, then doing something like
> "seq 10 | ./a.out"
Tests good on latest snapshot. Fails on
Cygwin DLL version info:
DLL version: 2.8.2
DLL epoch: 19
DLL old termios: 5
DLL malloc env: 28
Cygwin conv: 181
API major: 0
API minor: 313
Shared data: 5
DLL identifier: cygwin1
Mount registry: 3
Cygwin registry name: Cygwin
Installations name: Installations
Cygdrive default prefix:
Build date:
Shared id: cygwin1S5
> would drop some of the input lines.
> >>>
> >>> ...yep. It does. Specifically, I consistently only get
> the firsts
> >>> line:
> >>>
> >>> $ seq 10 | ./a.exe
> >>> got: 1
> >>>
> >>> $
> >>>
> >>> If I introduce a delay between the lines of stdin (which
> I tested by
> >>> just typing stdin from the keyboard), it works as expected.
> >>>
> >>> Looks like this one will need to go to the Cygwin mailing
> list; I'll
> >>> take it there shortly. Thank you all for your help
> getting this far!
> >>
> >> This is apparently fixed in cygwin v2.8.3 [see commit 78ade082fe,
> >> ('Revert "errno: Stop using _impure_ptr->_errno completely"',
> >> 19-07-2017), commit 9cc89b0438 ("cygwin: Use errno instead of
> >> _impure_ptr->_errno", 19-07-2017), commit a674199fc9
> ("cygwin: Bump
> >> DLL version to 2.8.3",
> >> 19-07-2017) and commit d2ae2f00b8 ("cygwin: add fflush fix
> to release
> >> notes", 19-07-2017)].
> >>
> >> I haven't had a chance to try v2.8.3 yet (it's 3am and I'm
> about to
> >> go get some sleep).
> >
> > Cygwin 2.8.3 hasn't been released yet,
>
> Heh, yes, I found that out myself this afternoon. ;-)
>
> > but I've just tested the
> > latest development snapshot with Jeff's simple test case,
> and it works
> > as expected, so I'm going to assume the Git test will start passing
> > once that version of the Cygwin DLL is released too.
>
> Hmm, I'm not keen on installing "snapshot"(s), so I think I
> will wait for the release to test it. (However, as a matter
> of interest, how would I obtain/install/test this snapshot
> release - is it a 'low-risk' exercise?)
Using https://cygwin.com/snapshots/x86_64/cygwin-20170823.tar.xz
D:\inst\cygwin\cygwin-20170823>usr\bin\bash.exe
bash-4.4$ seq 10 | ./a.exe
got: 1
got: 2
got: 3
got: 4
got: 5
got: 6
got: 7
got: 8
got: 9
got: 10
bash-4.4$ cat cygcheck.out
Cygwin Configuration Diagnostics
Current System Time: Sun Aug 27 14:35:25 2017
Windows 10 Professional Ver 10.0 Build 14393
Cygwin DLL version info:
DLL version: 2.9.0
DLL epoch: 19
DLL old termios: 5
DLL malloc env: 28
Cygwin conv: 181
API major: 0
API minor: 317
Shared data: 5
DLL identifier: cygwin1
Mount registry: 3
Cygwin registry name: Cygwin
Installations name: Installations
Cygdrive default prefix:
Build date:
Snapshot date: 20170823-15:44:28
Shared id: cygwin1S5
bash-4.4$
v/r,
Jason Pyeron
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
- -
- Jason Pyeron PD Inc. http://www.pdinc.us -
- Principal Consultant 10 West 24th Street #100-
- +1 (443) 269-1555 x333Baltimore, Maryland 21218 -
- -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Automatically delete branches containing accepted patches?
Hi, I have lots of git/git branches and once in a while some patches make it into git/git master. If this happens I would like to delete my branch with the patch automatically. That's not easily possible as the hashes on my branches are, of course, not the same as the hashes on git/git. How do you deal with this situation? Do you manually delete your branches or do you have some clever script to share? Thanks, Lars
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
On Sun, Aug 27, 2017 at 11:41:52AM -0400, Jeff King wrote: > Ouch. So this means that git since v2.11 is basically leaking every > non-byte pack sent by upload-pack (so all of the ref advertisement and > want/have negotiation). Determined experimentally that the answer is: yes. The increase in ref advertisement is roughly linear with the size of your packed-refs, so for most people this wasn't a big deal (and even if you have insanely large packed-refs, it's "only" 2-3x the RAM; potentially a problem, but only if you're close to the tipping point already). -Peff
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
> On 27 Aug 2017, at 09:37, Martin Ågren 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?
> We could keep the strbuf non-static and instead make sure we always
> release it before returning (but not before we die, so that we don't
> touch errno). That would also prepare us for threaded use. But until
> that needs to happen, let's just restore the static-ness so that we get
> back to a situation where we (eventually) do not continuosly keep
> allocating memory.
>
> Signed-off-by: Martin Ågren
> ---
> I waffled between "fixing the memory leak" by releasing the buffer and
> "fixing the static-ness" as in this patch. I can see arguments both ways
> and don't have any strong opinion.
>
> pkt-line.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/pkt-line.c b/pkt-line.c
> index 7db911957..f364944b9 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -136,9 +136,10 @@ static void format_packet(struct strbuf *out, const char
> *fmt, va_list args)
> static int packet_write_fmt_1(int fd, int gently,
> const char *fmt, va_list args)
> {
> - struct strbuf buf = STRBUF_INIT;
> + static struct strbuf buf = STRBUF_INIT;
> ssize_t count;
>
> + strbuf_reset(&buf);
> format_packet(&buf, fmt, args);
> count = write_in_full(fd, buf.buf, buf.len);
> if (count == buf.len)
> --
> 2.14.1.151.g45c1275a3.dirty
Looks good to me!
- Lars
Re: cat-file timing window on Cygwin
On 27/08/17 12:33, Adam Dinwoodie wrote:
> On Sun, Aug 27, 2017 at 03:06:31AM +0100, Ramsay Jones wrote:
>> On 26/08/17 22:11, Adam Dinwoodie wrote:
>>> On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:
Interesting. I find it a little hard to believe there's so obvious a bug
as "fflush(NULL) flushes stdin", but well...that's what it seems like.
If that's truly what it is, this is the minimal reproduction I came up
with:
-- >8 --
#include
int main(void)
{
char buf[256];
while (fgets(buf, sizeof(buf), stdin)) {
fprintf(stdout, "got: %s", buf);
fflush(NULL);
}
return 0;
}
-- 8< --
If this really is the bug, then doing something like "seq 10 | ./a.out"
would drop some of the input lines.
>>>
>>> ...yep. It does. Specifically, I consistently only get the firsts
>>> line:
>>>
>>> $ seq 10 | ./a.exe
>>> got: 1
>>>
>>> $
>>>
>>> If I introduce a delay between the lines of stdin (which I tested by
>>> just typing stdin from the keyboard), it works as expected.
>>>
>>> Looks like this one will need to go to the Cygwin mailing list; I'll
>>> take it there shortly. Thank you all for your help getting this far!
>>
>> This is apparently fixed in cygwin v2.8.3 [see commit 78ade082fe,
>> ('Revert "errno: Stop using _impure_ptr->_errno completely"', 19-07-2017),
>> commit 9cc89b0438 ("cygwin: Use errno instead of _impure_ptr->_errno",
>> 19-07-2017), commit a674199fc9 ("cygwin: Bump DLL version to 2.8.3",
>> 19-07-2017) and commit d2ae2f00b8 ("cygwin: add fflush fix to release
>> notes", 19-07-2017)].
>>
>> I haven't had a chance to try v2.8.3 yet (it's 3am and I'm about to
>> go get some sleep).
>
> Cygwin 2.8.3 hasn't been released yet,
Heh, yes, I found that out myself this afternoon. ;-)
> but I've just tested the latest
> development snapshot with Jeff's simple test case, and it works as
> expected, so I'm going to assume the Git test will start passing once
> that version of the Cygwin DLL is released too.
Hmm, I'm not keen on installing "snapshot"(s), so I think I will
wait for the release to test it. (However, as a matter of interest,
how would I obtain/install/test this snapshot release - is it a
'low-risk' exercise?)
ATB,
Ramsay Jones
Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
On Sun, Aug 27, 2017 at 09:37:32AM +0200, Martin Ågren 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.
>
> We could keep the strbuf non-static and instead make sure we always
> release it before returning (but not before we die, so that we don't
> touch errno). That would also prepare us for threaded use. But until
> that needs to happen, let's just restore the static-ness so that we get
> back to a situation where we (eventually) do not continuosly keep
> allocating memory.
Ouch. So this means that git since v2.11 is basically leaking every
non-byte pack sent by upload-pack (so all of the ref advertisement and
want/have negotiation).
I'm surprised nobody noticed the extra memory use, but I guess those
things aren't usually _too_ big.
> Signed-off-by: Martin Ågren
> ---
> I waffled between "fixing the memory leak" by releasing the buffer and
> "fixing the static-ness" as in this patch. I can see arguments both ways
> and don't have any strong opinion.
I think this is a good fix for now as it takes as back to the pre-bug
state. The only downside with the static buffer is that it's not
reentrant. Since the function is just inherently writing out the result
and then forgetting it, in a single thread there's no opportunity for a
sub-function to try writing another packet. And I don't think we have
any code paths that write packets from multiple threads. That may be
something we do eventually, but we can deal with it then (and until
then, it's nice to avoid the malloc/free overhead).
-Peff
Re: [GSoC][PATCH v3 0/4] Incremental rewrite of git-submodules
On Sat, Aug 26, 2017 at 12:21 AM, Junio C Hamano wrote: > Thanks. I'll try to queue these before I'll go offline. > > Mentors may want to help the student further in adjusting the patch > series to the more recent codebase; unfortunately the area the GSoC > project touches is a bit fluid these days. I resolved the conflicts > with nd/pune-in-worktree and bw/submodule-config-cleanup topics so > that the result would compile, but I am not sure if the resolution > is correct (e.g. there may be a new leak I introduced while doing > so). > Thanks. I'll see the dirty merges and will resend the whole series after reviewing the dirty merge and sending a new one with/without changes as required. Thanks, Prathamesh Chavan
Re: cat-file timing window on Cygwin
On Sun, Aug 27, 2017 at 03:06:31AM +0100, Ramsay Jones wrote:
> On 26/08/17 22:11, Adam Dinwoodie wrote:
> > On Sat, Aug 26, 2017 at 11:53:37AM -0700, Jeff King wrote:
> >> Interesting. I find it a little hard to believe there's so obvious a bug
> >> as "fflush(NULL) flushes stdin", but well...that's what it seems like.
> >>
> >> If that's truly what it is, this is the minimal reproduction I came up
> >> with:
> >>
> >> -- >8 --
> >> #include
> >>
> >> int main(void)
> >> {
> >>char buf[256];
> >>while (fgets(buf, sizeof(buf), stdin)) {
> >>fprintf(stdout, "got: %s", buf);
> >>fflush(NULL);
> >>}
> >>return 0;
> >> }
> >> -- 8< --
> >>
> >> If this really is the bug, then doing something like "seq 10 | ./a.out"
> >> would drop some of the input lines.
> >
> > ...yep. It does. Specifically, I consistently only get the firsts
> > line:
> >
> > $ seq 10 | ./a.exe
> > got: 1
> >
> > $
> >
> > If I introduce a delay between the lines of stdin (which I tested by
> > just typing stdin from the keyboard), it works as expected.
> >
> > Looks like this one will need to go to the Cygwin mailing list; I'll
> > take it there shortly. Thank you all for your help getting this far!
>
> This is apparently fixed in cygwin v2.8.3 [see commit 78ade082fe,
> ('Revert "errno: Stop using _impure_ptr->_errno completely"', 19-07-2017),
> commit 9cc89b0438 ("cygwin: Use errno instead of _impure_ptr->_errno",
> 19-07-2017), commit a674199fc9 ("cygwin: Bump DLL version to 2.8.3",
> 19-07-2017) and commit d2ae2f00b8 ("cygwin: add fflush fix to release
> notes", 19-07-2017)].
>
> I haven't had a chance to try v2.8.3 yet (it's 3am and I'm about to
> go get some sleep).
Cygwin 2.8.3 hasn't been released yet, but I've just tested the latest
development snapshot with Jeff's simple test case, and it works as
expected, so I'm going to assume the Git test will start passing once
that version of the Cygwin DLL is released too.
[PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()
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.
We could keep the strbuf non-static and instead make sure we always
release it before returning (but not before we die, so that we don't
touch errno). That would also prepare us for threaded use. But until
that needs to happen, let's just restore the static-ness so that we get
back to a situation where we (eventually) do not continuosly keep
allocating memory.
Signed-off-by: Martin Ågren
---
I waffled between "fixing the memory leak" by releasing the buffer and
"fixing the static-ness" as in this patch. I can see arguments both ways
and don't have any strong opinion.
pkt-line.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/pkt-line.c b/pkt-line.c
index 7db911957..f364944b9 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -136,9 +136,10 @@ static void format_packet(struct strbuf *out, const char
*fmt, va_list args)
static int packet_write_fmt_1(int fd, int gently,
const char *fmt, va_list args)
{
- struct strbuf buf = STRBUF_INIT;
+ static struct strbuf buf = STRBUF_INIT;
ssize_t count;
+ strbuf_reset(&buf);
format_packet(&buf, fmt, args);
count = write_in_full(fd, buf.buf, buf.len);
if (count == buf.len)
--
2.14.1.151.g45c1275a3.dirty
