Web lists-archives.com

Re: [PATCH v3 1/4] trace.c: introduce trace_run_command()




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