On Tue, Dec 21, 2010 at 3:24 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Nguyen Thai Ngoc Duy <pclouds@xxxxxxxxx> writes: > >> This is pretty much a clean-up patch from my perspective. Do we really >> need two paragraph explanation for marking a function static? > > I've already applied it, but I think it is much better to do this instead > (on top of Vasyl' Vavrychuk's patch). > > A more interesting topic is why the try-to-free-pack-memory logic needs to > be disabled in the first place. Â3a09425 (Do not call release_pack_memory > in malloc wrappers when GIT_TRACE is used, 2010-05-08) explains that it is > to avoid a race on Windows, and it looks like a workaround not a solution > ("can be called without locking"---"why aren't we locking then?"). > > Not that it matters in the context of "trace", which is a debugging > facility, that this is a workaround. > > -- >8 -- > Subject: set_try_to_free_routine(NULL) means "do nothing special" > > This way, the next caller that wants to disable our memory reclamation > machinery does not have to define its own do_nothing() stub. > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > Âtrace.c  |  Â8 ++------ > Âwrapper.c |  Â2 ++ > Â2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/trace.c b/trace.c > index 62586fa..0fb2a2c 100644 > --- a/trace.c > +++ b/trace.c > @@ -25,10 +25,6 @@ > Â#include "cache.h" > Â#include "quote.h" > > -static void do_nothing(size_t unused) > -{ > -} > - > Â/* Get a trace file descriptor from GIT_TRACE env variable. */ > Âstatic int get_trace_fd(int *need_close) > Â{ > @@ -76,7 +72,7 @@ void trace_printf(const char *fmt, ...) >    Âif (!fd) >        Âreturn; > > -    set_try_to_free_routine(do_nothing);  Â/* is never reset */ > +    set_try_to_free_routine(NULL); Â/* is never reset */ >    Âstrbuf_init(&buf, 64); >    Âva_start(ap, fmt); >    Âlen = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap); > @@ -108,7 +104,7 @@ void trace_argv_printf(const char **argv, const char *fmt, ...) >    Âif (!fd) >        Âreturn; > > -    set_try_to_free_routine(do_nothing);  Â/* is never reset */ > +    set_try_to_free_routine(NULL); Â/* is never reset */ >    Âstrbuf_init(&buf, 64); >    Âva_start(ap, fmt); >    Âlen = vsnprintf(buf.buf, strbuf_avail(&buf), fmt, ap); > diff --git a/wrapper.c b/wrapper.c > index 4c1639f..8d7dd31 100644 > --- a/wrapper.c > +++ b/wrapper.c > @@ -12,6 +12,8 @@ static void (*try_to_free_routine)(size_t size) = do_nothing; > Âtry_to_free_t set_try_to_free_routine(try_to_free_t routine) > Â{ >    Âtry_to_free_t old = try_to_free_routine; > +    if (!routine) > +        routine = do_nothing; Maybe I'm missing something, or I'm confused (or I do not understand what I'm reading), but how you are assign routine to do_nothing if you have removed do_nothing above? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html