Re: [PATCH 2/3] trace2: trim whitespace in start message in perf target format

[Junio C Hamano <gitster@pobox.com>]

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Trim leading/trailing whitespace from the command line
> printed in the "start" message in the perf target format.
>
> We use `sq_quote_argv_pretty()` to format the message
> and it adds a leading space to the output.  Trim that.

strbuf_trim() not just drops a single leading space, but removes
consecutive spaces from both ends.  But the first char after the SP
comes from the first arg, and it can never be a whitespace (as a
payload that begins with a whitespace will be quoted, so it will be
a single quote), and the last char in the buffer would also be
either a closing single quote (if the last argument ends with a
whitespace) or a non whitespace "safe" character, so it is safe to
use strbuf_trim() here.

I wonder if we want to lose the prepending of SP from
sq_quote_argv_pretty(), though:

 * run-command.c::trace_run_command() does rely on having SP there,
   so the caller needs adjusting if we did so.

 * trace.c::trace_argv_vprintf_fl() also needs SP there after the
   caller supplied format.

 * trace.c::print_command_performance_atexit() expects command_line
   begins with the extra SP left by the sq_quote_argv_pretty()
   called by the trace_command_performance(); the format string
   given to trace_performance_leave() there needs adjusting.


By the way, use of sq_quote_argv_pretty() in builtin/rebase.c on
opts->git_am_opts.argv done in run_specific_rebase() is dubious.
The argv array is made into a single string that safely uses sq,
appropriate to feed a shell.  But that string is passed as the
"value" parameter to add_var() helper that expects to receive a raw
value (hence it calls sq_quote_buf() on the value), resulting in a
string that is doubly quoted.  I am not sure if that was intended.

In any case, the patch itself obviously look correct.

> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>  trace2/tr2_tgt_perf.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c
> index 4a9d99218b..ed4e708f28 100644
> --- a/trace2/tr2_tgt_perf.c
> +++ b/trace2/tr2_tgt_perf.c
> @@ -185,6 +185,7 @@ static void fn_start_fl(const char *file, int line,
>  	struct strbuf buf_payload = STRBUF_INIT;
>  
>  	sq_quote_argv_pretty(&buf_payload, argv);
> +	strbuf_trim(&buf_payload);
>  
>  	perf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,
>  			 NULL, NULL, &buf_payload);