Re: [PATCH] trace.c: mark file-local function static

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]