{"thread":{"id":"26087","subject":"[PATCH] trace.c: mark file-local function static","startedAt":"2010-12-16T22:38:42Z","lastAt":"2010-12-21T19:54:25Z","messageCount":14,"participants":["Vasyl'","Thiago Farina","Drew Northup","Nguyen Thai Ngoc Duy","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"158238","messageId":"AANLkTinxJdASW6mQVU50grA2mUz6gt+gUND30VRK=BCN@mail.gmail.com","threadId":"26087","inReplyTo":null,"subject":"[PATCH] trace.c: mark file-local function static","fromName":"Vasyl'","fromEmail":"vvavrychuk@gmail.com","sentAt":"2010-12-16T22:38:42Z","receivedAt":"2010-12-16T22:38:42Z","isPatch":true,"sender":{"key":"vvavrychuk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/152563?v=4"},"body":"Signed-off-by: Vasyl' Vavrychuk \n---\n trace.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/trace.c b/trace.c\nindex 1e560cb..62586fa 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -25,7 +25,7 @@\n #include \"cache.h\"\n #include \"quote.h\"\n\n-void do_nothing(size_t unused)\n+static void do_nothing(size_t unused)\n {\n }\n\n-- \n1.7.1\n"},{"id":"158243","messageId":"AANLkTimBtpOx_GBzC=g4V6jW2aiF7Hg8uALWt2NQFFZG@mail.gmail.com","threadId":"26087","inReplyTo":"AANLkTinxJdASW6mQVU50grA2mUz6gt+gUND30VRK=BCN@mail.gmail.com","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-16T23:43:50Z","receivedAt":"2010-12-16T23:43:50Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Thu, Dec 16, 2010 at 8:38 PM, Vasyl' wrote:\n> Signed-off-by: Vasyl' Vavrychuk \n> ---\n>  trace.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/trace.c b/trace.c\n> index 1e560cb..62586fa 100644\n> --- a/trace.c\n> +++ b/trace.c\n> @@ -25,7 +25,7 @@\n>  #include \"cache.h\"\n>  #include \"quote.h\"\n>\n> -void do_nothing(size_t unused)\n> +static void do_nothing(size_t unused)\n>  {\n>  }\n>\nIf it means something, this looks sane to me.\n\nAcked-by: Thiago Farina \n\n> --\n> 1.7.1\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"158395","messageId":"1292846433.19322.1.camel@drew-northup.unet.maine.edu","threadId":"26087","inReplyTo":"AANLkTimBtpOx_GBzC=g4V6jW2aiF7Hg8uALWt2NQFFZG@mail.gmail.com","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Drew Northup","fromEmail":"drew.northup@maine.edu","sentAt":"2010-12-20T12:00:33Z","receivedAt":"2010-12-20T12:00:33Z","isPatch":true,"sender":{"key":"drew.northup@maine.edu","avatar":"https://avatars.githubusercontent.com/u/18331571?v=4"},"body":"\nOn Thu, 2010-12-16 at 21:43 -0200, Thiago Farina wrote:\n> On Thu, Dec 16, 2010 at 8:38 PM, Vasyl' wrote:\n> > Signed-off-by: Vasyl' Vavrychuk \n> > ---\n> > trace.c | 2 +-\n> > 1 files changed, 1 insertions(+), 1 deletions(-)\n> >\n> > diff --git a/trace.c b/trace.c\n> > index 1e560cb..62586fa 100644\n> > --- a/trace.c\n> > +++ b/trace.c\n> > @@ -25,7 +25,7 @@\n> > #include \"cache.h\"\n> > #include \"quote.h\"\n> >\n> > -void do_nothing(size_t unused)\n> > +static void do_nothing(size_t unused)\n> > {\n> > }\n> >\n> If it means something, this looks sane to me.\n> \n> Acked-by: Thiago Farina \n\nIt may be sane, but why should we trust that it is without a commit\nmessage?\n\n-- \n-Drew Northup N1XIM\n AKA RvnPhnx on OPN\n________________________________________________\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"158399","messageId":"AANLkTimkxJHvTAvra+B-0bAQopd8s21ZrFCPG_ALbZZ9@mail.gmail.com","threadId":"26087","inReplyTo":"1292846433.19322.1.camel@drew-northup.unet.maine.edu","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-20T15:17:49Z","receivedAt":"2010-12-20T15:17:49Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Mon, Dec 20, 2010 at 10:00 AM, Drew Northup wrote:\n>\n> On Thu, 2010-12-16 at 21:43 -0200, Thiago Farina wrote:\n>> On Thu, Dec 16, 2010 at 8:38 PM, Vasyl' wrote:\n>> > Signed-off-by: Vasyl' Vavrychuk \n>> > ---\n>> >  trace.c |    2 +-\n>> >  1 files changed, 1 insertions(+), 1 deletions(-)\n>> >\n>> > diff --git a/trace.c b/trace.c\n>> > index 1e560cb..62586fa 100644\n>> > --- a/trace.c\n>> > +++ b/trace.c\n>> > @@ -25,7 +25,7 @@\n>> >  #include \"cache.h\"\n>> >  #include \"quote.h\"\n>> >\n>> > -void do_nothing(size_t unused)\n>> > +static void do_nothing(size_t unused)\n>> >  {\n>> >  }\n>> >\n>> If it means something, this looks sane to me.\n>>\n>> Acked-by: Thiago Farina \n>\n> It may be sane, but why should we trust that it is without a commit\n> message?\n\nWhy such trivial thing needs further explanation?\n"},{"id":"158401","messageId":"1292863989.19322.27.camel@drew-northup.unet.maine.edu","threadId":"26087","inReplyTo":"AANLkTimkxJHvTAvra+B-0bAQopd8s21ZrFCPG_ALbZZ9@mail.gmail.com","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Drew Northup","fromEmail":"drew.northup@maine.edu","sentAt":"2010-12-20T16:53:09Z","receivedAt":"2010-12-20T16:53:09Z","isPatch":true,"sender":{"key":"drew.northup@maine.edu","avatar":"https://avatars.githubusercontent.com/u/18331571?v=4"},"body":"\nOn Mon, 2010-12-20 at 13:17 -0200, Thiago Farina wrote:\n> On Mon, Dec 20, 2010 at 10:00 AM, Drew Northup wrote:\n> >\n> > On Thu, 2010-12-16 at 21:43 -0200, Thiago Farina wrote:\n> >> On Thu, Dec 16, 2010 at 8:38 PM, Vasyl' wrote:\n> >> > Signed-off-by: Vasyl' Vavrychuk \n> >> > ---\n> >> > trace.c | 2 +-\n> >> > 1 files changed, 1 insertions(+), 1 deletions(-)\n> >> >\n> >> > diff --git a/trace.c b/trace.c\n> >> > index 1e560cb..62586fa 100644\n> >> > --- a/trace.c\n> >> > +++ b/trace.c\n> >> > @@ -25,7 +25,7 @@\n> >> > #include \"cache.h\"\n> >> > #include \"quote.h\"\n> >> >\n> >> > -void do_nothing(size_t unused)\n> >> > +static void do_nothing(size_t unused)\n> >> > {\n> >> > }\n> >> >\n> >> If it means something, this looks sane to me.\n> >>\n> >> Acked-by: Thiago Farina \n> >\n> > It may be sane, but why should we trust that it is without a commit\n> > message?\n> \n> Why such trivial thing needs further explanation?\n\nBecause even trivial fixes may break non-trivial things. \nIn addition, without justification we'd just as soon have somebody come\nback with another patch six months down the road that changes it back to\nthe original code. Now that wouldn't make a whole lot of sense, now\nwould it?\nAlas the best way to avoid such a situation is to explain why a change\nwas made to begin with.\n\n-- \n-Drew Northup N1XIM\n AKA RvnPhnx on OPN\n________________________________________________\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"158421","messageId":"AANLkTin2QuMF93RqrRcScxvkzhU4OFJ9Nt42tV+FOjxS@mail.gmail.com","threadId":"26087","inReplyTo":"1292863989.19322.27.camel@drew-northup.unet.maine.edu","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-21T00:28:41Z","receivedAt":"2010-12-21T00:28:41Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Mon, Dec 20, 2010 at 2:53 PM, Drew Northup wrote:\n>\n> On Mon, 2010-12-20 at 13:17 -0200, Thiago Farina wrote:\n>> On Mon, Dec 20, 2010 at 10:00 AM, Drew Northup wrote:\n>> >\n>> > On Thu, 2010-12-16 at 21:43 -0200, Thiago Farina wrote:\n>> >> On Thu, Dec 16, 2010 at 8:38 PM, Vasyl' wrote:\n>> >> > Signed-off-by: Vasyl' Vavrychuk \n>> >> > ---\n>> >> >  trace.c |    2 +-\n>> >> >  1 files changed, 1 insertions(+), 1 deletions(-)\n>> >> >\n>> >> > diff --git a/trace.c b/trace.c\n>> >> > index 1e560cb..62586fa 100644\n>> >> > --- a/trace.c\n>> >> > +++ b/trace.c\n>> >> > @@ -25,7 +25,7 @@\n>> >> >  #include \"cache.h\"\n>> >> >  #include \"quote.h\"\n>> >> >\n>> >> > -void do_nothing(size_t unused)\n>> >> > +static void do_nothing(size_t unused)\n>> >> >  {\n>> >> >  }\n>> >> >\n>> >> If it means something, this looks sane to me.\n>> >>\n>> >> Acked-by: Thiago Farina \n>> >\n>> > It may be sane, but why should we trust that it is without a commit\n>> > message?\n>>\n>> Why such trivial thing needs further explanation?\n>\n> Because even trivial fixes may break non-trivial things.\n> In addition, without justification we'd just as soon have somebody come\n> back with another patch six months down the road that changes it back to\n> the original code. Now that wouldn't make a whole lot of sense, now\n> would it?\n\nI don't think so, it's making the function private, because the\nfunction is used only in that file and as such if you see a function\nmarked as static you know that and doesn't need further explanation in\nmy pov (but it seems you don't think like that).\n\n> Alas the best way to avoid such a situation is to explain why a change\n> was made to begin with.\n>\n\nSo, you are welcome to contribute and suggest such description for\nthis trivial (that may break non-trivial things) patch. So we can\nplease you and others in the future.\n\n\nThanks.\n"},{"id":"158431","messageId":"1292934581.26698.14.camel@drew-northup.unet.maine.edu","threadId":"26087","inReplyTo":"AANLkTin2QuMF93RqrRcScxvkzhU4OFJ9Nt42tV+FOjxS@mail.gmail.com","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Drew Northup","fromEmail":"drew.northup@maine.edu","sentAt":"2010-12-21T12:29:41Z","receivedAt":"2010-12-21T12:29:41Z","isPatch":true,"sender":{"key":"drew.northup@maine.edu","avatar":"https://avatars.githubusercontent.com/u/18331571?v=4"},"body":"\nOn Mon, 2010-12-20 at 22:28 -0200, Thiago Farina wrote:\n> On Mon, Dec 20, 2010 at 2:53 PM, Drew Northup wrote:\n> >\n> > On Mon, 2010-12-20 at 13:17 -0200, Thiago Farina wrote:\n> >> On Mon, Dec 20, 2010 at 10:00 AM, Drew Northup wrote:\n> >> >\n> >> > On Thu, 2010-12-16 at 21:43 -0200, Thiago Farina wrote:\n> >> >> On Thu, Dec 16, 2010 at 8:38 PM, Vasyl' wrote:\n> >> >> > Signed-off-by: Vasyl' Vavrychuk \n> >> >> > ---\n> >> >> > trace.c | 2 +-\n> >> >> > 1 files changed, 1 insertions(+), 1 deletions(-)\n> >> >> >\n> >> >> > diff --git a/trace.c b/trace.c\n> >> >> > index 1e560cb..62586fa 100644\n> >> >> > --- a/trace.c\n> >> >> > +++ b/trace.c\n> >> >> > @@ -25,7 +25,7 @@\n> >> >> > #include \"cache.h\"\n> >> >> > #include \"quote.h\"\n> >> >> >\n> >> >> > -void do_nothing(size_t unused)\n> >> >> > +static void do_nothing(size_t unused)\n> >> >> > {\n> >> >> > }\n> >> >> >\n> >> >> If it means something, this looks sane to me.\n> >> >>\n> >> >> Acked-by: Thiago Farina \n> >> >\n> >> > It may be sane, but why should we trust that it is without a commit\n> >> > message?\n> >>\n> >> Why such trivial thing needs further explanation?\n> >\n> > Because even trivial fixes may break non-trivial things.\n> > In addition, without justification we'd just as soon have somebody come\n> > back with another patch six months down the road that changes it back to\n> > the original code. Now that wouldn't make a whole lot of sense, now\n> > would it?\n> \n> I don't think so, it's making the function private, because the\n> function is used only in that file and as such if you see a function\n> marked as static you know that and doesn't need further explanation in\n> my pov (but it seems you don't think like that).\n\nAll the patch above does is tell the compiler to enforce compilation of\nthat function as static. That is most definitely not the same thing as\nmaking it private. (I think we can agree that it is being used as if it\nwere being enforced as private at some meta-level, but the compiler\nisn't going to enforce it for us...) While I think the code change is\nfairly clear, as I said earlier: without a commit message we don't have\na good reason for not making it non-static again later on, flip-flopping\nad-infinitum.\n\nCommit messages for isolated changes such as this build up a story, if\nyou will, providing future contributors with insight as to why the group\nmade a change when it did--even when the change is minor (in fact often\nmost importantly when the change is minor)--by putting it in context.\n\n> > Alas the best way to avoid such a situation is to explain why a change\n> > was made to begin with.\n> >\n> \n> So, you are welcome to contribute and suggest such description for\n> this trivial (that may break non-trivial things) patch. So we can\n> please you and others in the future.\n\nAs I am complaining that I don't know what the submitter was thinking\nthat sounds particularly odd to me. How I am supposed to describe for\nthe group what the commit's author was thinking in a commit message that\nI would like to see added to a patch when in fact the whole problem is\nthat I don't know specifically what he was thinking? \"Changed function\nto static for no known reason whatsoever\" isn't much of a commit\nmessage.\n\n-- \n-Drew Northup N1XIM\n AKA RvnPhnx on OPN\n________________________________________________\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"158438","messageId":"AANLkTinL6RytF=x7a8URoxGQoLHhx0ip7gRNMp+nRaNB@mail.gmail.com","threadId":"26087","inReplyTo":"1292863989.19322.27.camel@drew-northup.unet.maine.edu","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-12-21T14:52:59Z","receivedAt":"2010-12-21T14:52:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Dec 20, 2010 at 11:53 PM, Drew Northup wrote:\n>> Why such trivial thing needs further explanation?\n>\n> Because even trivial fixes may break non-trivial things.\n\nThe only thing it could break is compilation.\n\n> In addition, without justification we'd just as soon have somebody come\n> back with another patch six months down the road that changes it back to\n> the original code. Now that wouldn't make a whole lot of sense, now\n> would it?\n\nYeah I would expect somebody reverting this patch _if_ this function\nis useful outside trace.c again. On the other hand, this patch saves\ndo_nothing from global namespace so somewhere, some time, somebody can\nuse it.\n\nThis is pretty much a clean-up patch from my perspective. Do we really\nneed two paragraph explanation for marking a function static?\n-- \nDuy\n"},{"id":"158439","messageId":"1292944750.26698.16.camel@drew-northup.unet.maine.edu","threadId":"26087","inReplyTo":"AANLkTinL6RytF=x7a8URoxGQoLHhx0ip7gRNMp+nRaNB@mail.gmail.com","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Drew Northup","fromEmail":"drew.northup@maine.edu","sentAt":"2010-12-21T15:19:10Z","receivedAt":"2010-12-21T15:19:10Z","isPatch":true,"sender":{"key":"drew.northup@maine.edu","avatar":"https://avatars.githubusercontent.com/u/18331571?v=4"},"body":"\nOn Tue, 2010-12-21 at 21:52 +0700, Nguyen Thai Ngoc Duy wrote:\n> On Mon, Dec 20, 2010 at 11:53 PM, Drew Northup wrote:\n\n> Yeah I would expect somebody reverting this patch _if_ this function\n> is useful outside trace.c again. On the other hand, this patch saves\n> do_nothing from global namespace so somewhere, some time, somebody can\n> use it.\n> \n> This is pretty much a clean-up patch from my perspective. Do we really\n> need two paragraph explanation for marking a function static?\n\nNo, but 1 sentence would be dandy.\n\n-- \n-Drew Northup N1XIM\n AKA RvnPhnx on OPN\n________________________________________________\n\"As opposed to vegetable or mineral error?\"\n-John Pescatore, SANS NewsBites Vol. 12 Num. 59\n"},{"id":"158440","messageId":"AANLkTimZ=xBCewESqQmOfOggkAOvqnZfavU2suLT6ATJ@mail.gmail.com","threadId":"26087","inReplyTo":"1292934581.26698.14.camel@drew-northup.unet.maine.edu","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-21T15:59:31Z","receivedAt":"2010-12-21T15:59:31Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Tue, Dec 21, 2010 at 10:29 AM, Drew Northup wrote:\n> While I think the code change is fairly clear, as I said earlier: without a commit message we don't have\n> a good reason for not making it non-static again later on, flip-flopping\n> ad-infinitum.\n>\n\nWhy you are complain about a one word change at all? Are you the\nmaintainer? Are you going to make a change that uses do_nothing in\nfuture? If for some reason someone needs to use this function in other\nplace, the problem is of this person not you, he you will have to\nargue and defend his change for whatever reasons he has, and will be a\nproblem for the maintainer not for you.\n\n\n> Commit messages for isolated changes such as this build up a story, if\n> you will, providing future contributors with insight as to why the group\n> made a change when it did--even when the change is minor (in fact often\n> most importantly when the change is minor)--by putting it in context.\n>\nWe all know that, and Junio is doing a good job enforcing GOOD commit\nmessages. But in this case this is STUPID in my pov (others can and\nprobably will disagree with me, but I don't care).\n\n>> > Alas the best way to avoid such a situation is to explain why a change\n>> > was made to begin with.\n>> >\n>>\n>> So, you are welcome to contribute and suggest such description for\n>> this trivial (that may break non-trivial things) patch. So we can\n>> please you and others in the future.\n>\n> As I am complaining that I don't know what the submitter was thinking\n> that sounds particularly odd to me. How I am supposed to describe for\n> the group what the commit's author was thinking in a commit message that\n> I would like to see added to a patch when in fact the whole problem is\n> that I don't know specifically what he was thinking?\n\nWell, what the hell we are talking about so? If you are arguing for a\ncommit message, I would expect that you would have a suggestion, at\nleast to show why you are so interested in this particular patch.\n\nAnd many contributors here, besides suggesting BETTER commit messages,\nthey help others with how the patch could be written, and you are not\nhelping in either way.\n"},{"id":"158442","messageId":"7vbp4f9gzh.fsf@alter.siamese.dyndns.org","threadId":"26087","inReplyTo":"AANLkTinL6RytF=x7a8URoxGQoLHhx0ip7gRNMp+nRaNB@mail.gmail.com","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-21T17:24:18Z","receivedAt":"2010-12-21T17:24:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> This is pretty much a clean-up patch from my perspective. Do we really\n> need two paragraph explanation for marking a function static?\n\nI've already applied it, but I think it is much better to do this instead\n(on top of Vasyl' Vavrychuk's patch).\n\nA more interesting topic is why the try-to-free-pack-memory logic needs to\nbe disabled in the first place. 3a09425 (Do not call release_pack_memory\nin malloc wrappers when GIT_TRACE is used, 2010-05-08) explains that it is\nto avoid a race on Windows, and it looks like a workaround not a solution\n(\"can be called without locking\"---\"why aren't we locking then?\").\n\nNot that it matters in the context of \"trace\", which is a debugging\nfacility, that this is a workaround.\n\n-- >8 --\nSubject: set_try_to_free_routine(NULL) means \"do nothing special\"\n\nThis way, the next caller that wants to disable our memory reclamation\nmachinery does not have to define its own do_nothing() stub.\n\nSigned-off-by: Junio C Hamano \n---\n trace.c | 8 ++------\n wrapper.c | 2 ++\n 2 files changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/trace.c b/trace.c\nindex 62586fa..0fb2a2c 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -25,10 +25,6 @@\n #include \"cache.h\"\n #include \"quote.h\"\n \n-static void do_nothing(size_t unused)\n-{\n-}\n-\n /* Get a trace file descriptor from GIT_TRACE env variable. */\n static int get_trace_fd(int *need_close)\n {\n@@ -76,7 +72,7 @@ void trace_printf(const char *fmt, ...)\n \tif (!fd)\n \t\treturn;\n \n-\tset_try_to_free_routine(do_nothing);\t/* is never reset */\n+\tset_try_to_free_routine(NULL);\t/* is never reset */\n \tstrbuf_init(&buf, 64);\n \tva_start(ap, fmt);\n \tlen = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);\n@@ -108,7 +104,7 @@ void trace_argv_printf(const char **argv, const char *fmt, ...)\n \tif (!fd)\n \t\treturn;\n \n-\tset_try_to_free_routine(do_nothing);\t/* is never reset */\n+\tset_try_to_free_routine(NULL);\t/* is never reset */\n \tstrbuf_init(&buf, 64);\n \tva_start(ap, fmt);\n \tlen = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);\ndiff --git a/wrapper.c b/wrapper.c\nindex 4c1639f..8d7dd31 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -12,6 +12,8 @@ static void (*try_to_free_routine)(size_t size) = do_nothing;\n try_to_free_t set_try_to_free_routine(try_to_free_t routine)\n {\n \ttry_to_free_t old = try_to_free_routine;\n+\tif (!routine)\n+\t\troutine = do_nothing;\n \ttry_to_free_routine = routine;\n \treturn old;\n }\n"},{"id":"158443","messageId":"AANLkTikvV1chYnSJUMGGoHTFv8EaXuK5B7Vo71jVyTRy@mail.gmail.com","threadId":"26087","inReplyTo":"7vbp4f9gzh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-12-21T17:40:56Z","receivedAt":"2010-12-21T17:40:56Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Tue, Dec 21, 2010 at 3:24 PM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> This is pretty much a clean-up patch from my perspective. Do we really\n>> need two paragraph explanation for marking a function static?\n>\n> I've already applied it, but I think it is much better to do this instead\n> (on top of Vasyl' Vavrychuk's patch).\n>\n> A more interesting topic is why the try-to-free-pack-memory logic needs to\n> be disabled in the first place.  3a09425 (Do not call release_pack_memory\n> in malloc wrappers when GIT_TRACE is used, 2010-05-08) explains that it is\n> to avoid a race on Windows, and it looks like a workaround not a solution\n> (\"can be called without locking\"---\"why aren't we locking then?\").\n>\n> Not that it matters in the context of \"trace\", which is a debugging\n> facility, that this is a workaround.\n>\n> -- >8 --\n> Subject: set_try_to_free_routine(NULL) means \"do nothing special\"\n>\n> This way, the next caller that wants to disable our memory reclamation\n> machinery does not have to define its own do_nothing() stub.\n>\n> Signed-off-by: Junio C Hamano \n> ---\n>  trace.c   |    8 ++------\n>  wrapper.c |    2 ++\n>  2 files changed, 4 insertions(+), 6 deletions(-)\n>\n> diff --git a/trace.c b/trace.c\n> index 62586fa..0fb2a2c 100644\n> --- a/trace.c\n> +++ b/trace.c\n> @@ -25,10 +25,6 @@\n>  #include \"cache.h\"\n>  #include \"quote.h\"\n>\n> -static void do_nothing(size_t unused)\n> -{\n> -}\n> -\n>  /* Get a trace file descriptor from GIT_TRACE env variable. */\n>  static int get_trace_fd(int *need_close)\n>  {\n> @@ -76,7 +72,7 @@ void trace_printf(const char *fmt, ...)\n>        if (!fd)\n>                return;\n>\n> -       set_try_to_free_routine(do_nothing);    /* is never reset */\n> +       set_try_to_free_routine(NULL);  /* is never reset */\n>        strbuf_init(&buf, 64);\n>        va_start(ap, fmt);\n>        len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);\n> @@ -108,7 +104,7 @@ void trace_argv_printf(const char **argv, const char *fmt, ...)\n>        if (!fd)\n>                return;\n>\n> -       set_try_to_free_routine(do_nothing);    /* is never reset */\n> +       set_try_to_free_routine(NULL);  /* is never reset */\n>        strbuf_init(&buf, 64);\n>        va_start(ap, fmt);\n>        len = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap);\n> diff --git a/wrapper.c b/wrapper.c\n> index 4c1639f..8d7dd31 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -12,6 +12,8 @@ static void (*try_to_free_routine)(size_t size) = do_nothing;\n>  try_to_free_t set_try_to_free_routine(try_to_free_t routine)\n>  {\n>        try_to_free_t old = try_to_free_routine;\n> +       if (!routine)\n> +               routine = do_nothing;\n\nMaybe I'm missing something, or I'm confused (or I do not understand\nwhat I'm reading), but how you are assign routine to do_nothing if you\nhave removed do_nothing above?\n"},{"id":"158444","messageId":"7v4oa79fr3.fsf@alter.siamese.dyndns.org","threadId":"26087","inReplyTo":"AANLkTikvV1chYnSJUMGGoHTFv8EaXuK5B7Vo71jVyTRy@mail.gmail.com","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-21T17:50:56Z","receivedAt":"2010-12-21T17:50:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina writes:\n\n> Maybe I'm missing something, or I'm confused (or I do not understand\n> what I'm reading), but how you are assign routine to do_nothing if you\n> have removed do_nothing above?\n\nApply the patch and read the first twenty lines of resulting wrapper.c\n"},{"id":"158450","messageId":"201012212054.27054.j6t@kdbg.org","threadId":"26087","inReplyTo":"7vbp4f9gzh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] trace.c: mark file-local function static","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-12-21T19:54:25Z","receivedAt":"2010-12-21T19:54:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Dienstag, 21. Dezember 2010, Junio C Hamano wrote:\n> A more interesting topic is why the try-to-free-pack-memory logic needs to\n> be disabled in the first place. 3a09425 (Do not call release_pack_memory\n> in malloc wrappers when GIT_TRACE is used, 2010-05-08) explains that it is\n> to avoid a race on Windows, and it looks like a workaround not a solution\n> (\"can be called without locking\"---\"why aren't we locking then?\").\n>\n> Not that it matters in the context of \"trace\", which is a debugging\n> facility, that this is a workaround.\n\nExactly. A clean implementation is not worth the effort for this debugging \nfacility.\n\nBTW, these days it is not just Windows that is affected because we use threads \nin start_async everywhere if possible.\n\n-- Hannes\n"}]}