On Fri, Jan 12, 2018 at 04:56:04PM +0700, Nguyễn Thái Ngọc Duy wrote:
> This is the same as the old code that uses trace_argv_printf() in
> run-command.c. This function will be improved in later patches to
> print more information from struct child_process.
>
> A slight regression: the old code would print run-command.c:xxx as the
> trace location site while the new code prints trace.c:xxx. This should
> be fine until the second call site is added, then we might need a macro
> wrapper named trace_run_command() to capture the right source location.
I agree that's probably fine for now.
> +void trace_run_command(const struct child_process *cp)
> +{
> + struct strbuf buf = STRBUF_INIT;
> +
> + if (!prepare_trace_line(__FILE__, __LINE__,
> + &trace_default_key, &buf))
> + return;
> +
> + strbuf_addf(&buf, "trace: run_command:");
> +
> + sq_quote_argv(&buf, cp->argv, 0);
> + print_trace_line(&trace_default_key, &buf);
> +}
It looks like this leaks "buf".
If prepare_trace_line() returns 0, I think it's safe to assume that
nothing was allocated. So we'd just need a strbuf_release() at the end.
Looking at the other trace functions, it looks like a bunch of them have
the same problem.
-Peff
