{"thread":{"id":"51565","subject":"[PATCH 0/3] trace2: clean up formatting in perf target format","startedAt":"2019-07-31T20:04:31Z","lastAt":"2019-08-09T18:17:36Z","messageCount":33,"participants":["Jeff Hostetler via GitGitGadget","Junio C Hamano","Josh Steadmon","Jeff Hostetler","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"379707","messageId":"pull.298.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":null,"subject":"[PATCH 0/3] trace2: clean up formatting in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-31T20:04:28Z","receivedAt":"2019-07-31T20:04:31Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"This patch series contains a few column alignment and whitespace fixes for\nperf target format. This should make it a little easier to read.\n\nJeff Hostetler (3):\n trace2: cleanup column alignment in perf target format\n trace2: trim whitespace in start message in perf target format\n trace2: trim whitespace in region messages in perf target format\n\n trace2/tr2_tgt_perf.c | 42 ++++++++++++++++++++++++++++++++----------\n 1 file changed, 32 insertions(+), 10 deletions(-)\n\n\nbase-commit: 026dd738a6e5f1e42ef0f390feacb5ed6acc4ee8\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-298%2Fjeffhostetler%2Fupstream-2230-cleanup-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-298/jeffhostetler/upstream-2230-cleanup-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/298\n-- \ngitgitgadget\n"},{"id":"379708","messageId":"38b53da31f174892b564564e0a380c0a154c8f88.1564603467.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.git.gitgitgadget@gmail.com","subject":"[PATCH 1/3] trace2: cleanup column alignment in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-31T20:04:28Z","receivedAt":"2019-07-31T20:04:35Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTruncate/elide very long \"filename:linenumber\" field.\nTruncate region and data \"category\" field if necessary.\nAdjust overall column widths.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_perf.c | 39 +++++++++++++++++++++++++++++----------\n 1 file changed, 29 insertions(+), 10 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex ea0cbbe13e..4a9d99218b 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -21,10 +21,10 @@ static struct tr2_dst tr2dst_perf = { TR2_SYSENV_PERF, 0, 0, 0 };\n */\n static int tr2env_perf_be_brief;\n \n-#define TR2FMT_PERF_FL_WIDTH (50)\n+#define TR2FMT_PERF_FL_WIDTH (28)\n #define TR2FMT_PERF_MAX_EVENT_NAME (12)\n-#define TR2FMT_PERF_REPO_WIDTH (4)\n-#define TR2FMT_PERF_CATEGORY_WIDTH (10)\n+#define TR2FMT_PERF_REPO_WIDTH (3)\n+#define TR2FMT_PERF_CATEGORY_WIDTH (12)\n \n #define TR2_DOTS_BUFFER_SIZE (100)\n #define TR2_INDENT (2)\n@@ -79,17 +79,36 @@ static void perf_fmt_prepare(const char *event_name,\n \n \tif (!tr2env_perf_be_brief) {\n \t\tstruct tr2_tbuf tb_now;\n+\t\tsize_t fl_end_col;\n \n \t\ttr2_tbuf_local_time(&tb_now);\n \t\tstrbuf_addstr(buf, tb_now.buf);\n \t\tstrbuf_addch(buf, ' ');\n \n-\t\tif (file && *file)\n-\t\t\tstrbuf_addf(buf, \"%s:%d \", file, line);\n-\t\twhile (buf->len < TR2FMT_PERF_FL_WIDTH)\n+\t\tfl_end_col = buf->len + TR2FMT_PERF_FL_WIDTH;\n+\n+\t\tif (file && *file) {\n+\t\t\tstruct strbuf buf_fl = STRBUF_INIT;\n+\n+\t\t\tstrbuf_addf(&buf_fl, \"%s:%d\", file, line);\n+\n+\t\t\tif (buf_fl.len <= TR2FMT_PERF_FL_WIDTH)\n+\t\t\t\tstrbuf_addbuf(buf, &buf_fl);\n+\t\t\telse {\n+\t\t\t\tsize_t avail = TR2FMT_PERF_FL_WIDTH - 3;\n+\t\t\t\tstrbuf_addstr(buf, \"...\");\n+\t\t\t\tstrbuf_add(buf,\n+\t\t\t\t\t &buf_fl.buf[buf_fl.len - avail],\n+\t\t\t\t\t avail);\n+\t\t\t}\n+\n+\t\t\tstrbuf_release(&buf_fl);\n+\t\t}\n+\n+\t\twhile (buf->len < fl_end_col)\n \t\t\tstrbuf_addch(buf, ' ');\n \n-\t\tstrbuf_addstr(buf, \"| \");\n+\t\tstrbuf_addstr(buf, \" | \");\n \t}\n \n \tstrbuf_addf(buf, \"d%d | \", tr2_sid_depth());\n@@ -102,7 +121,7 @@ static void perf_fmt_prepare(const char *event_name,\n \t\tstrbuf_addf(buf, \"r%d \", repo->trace2_repo_id);\n \twhile (buf->len < len)\n \t\tstrbuf_addch(buf, ' ');\n-\tstrbuf_addstr(buf, \"| \");\n+\tstrbuf_addstr(buf, \" | \");\n \n \tif (p_us_elapsed_absolute)\n \t\tstrbuf_addf(buf, \"%9.6f | \",\n@@ -116,8 +135,8 @@ static void perf_fmt_prepare(const char *event_name,\n \telse\n \t\tstrbuf_addf(buf, \"%9s | \", \" \");\n \n-\tstrbuf_addf(buf, \"%-*s | \", TR2FMT_PERF_CATEGORY_WIDTH,\n-\t\t (category ? category : \"\"));\n+\tstrbuf_addf(buf, \"%-*.*s | \", TR2FMT_PERF_CATEGORY_WIDTH,\n+\t\t TR2FMT_PERF_CATEGORY_WIDTH, (category ? category : \"\"));\n \n \tif (ctx->nr_open_regions > 0) {\n \t\tint len_indent = TR2_INDENT_LENGTH(ctx);\n-- \ngitgitgadget\n\n"},{"id":"379709","messageId":"d2f7cf390853e72a6116eb54084ae4a38b6d8aba.1564603467.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.git.gitgitgadget@gmail.com","subject":"[PATCH 3/3] trace2: trim whitespace in region messages in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-31T20:04:30Z","receivedAt":"2019-07-31T20:04:35Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTrim trailing whitespace in \"region_enter\" and \"region_leave\"\nmessages in perf target format.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_perf.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex ed4e708f28..3c7ffbeb8a 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -455,6 +455,7 @@ static void fn_region_enter_printf_va_fl(const char *file, int line,\n \tif (label)\n \t\tstrbuf_addf(&buf_payload, \"label:%s \", label);\n \tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\tstrbuf_rtrim(&buf_payload);\n \n \tperf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,\n \t\t\t NULL, category, &buf_payload);\n@@ -472,6 +473,7 @@ static void fn_region_leave_printf_va_fl(\n \tif (label)\n \t\tstrbuf_addf(&buf_payload, \"label:%s \", label);\n \tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\tstrbuf_rtrim(&buf_payload);\n \n \tperf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,\n \t\t\t &us_elapsed_region, category, &buf_payload);\n-- \ngitgitgadget\n"},{"id":"379710","messageId":"834ee95cd2fe919c7a5a2d3cc3d647cfdeebe9e6.1564603467.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.git.gitgitgadget@gmail.com","subject":"[PATCH 2/3] trace2: trim whitespace in start message in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-31T20:04:29Z","receivedAt":"2019-07-31T20:04:41Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTrim leading/trailing whitespace from the command line\nprinted in the \"start\" message in the perf target format.\n\nWe use `sq_quote_argv_pretty()` to format the message\nand it adds a leading space to the output. Trim that.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_perf.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex 4a9d99218b..ed4e708f28 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -185,6 +185,7 @@ static void fn_start_fl(const char *file, int line,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tsq_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_trim(&buf_payload);\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n-- \ngitgitgadget\n\n"},{"id":"379778","messageId":"xmqqef24muxe.fsf@gitster-ct.c.googlers.com","threadId":"51565","inReplyTo":"834ee95cd2fe919c7a5a2d3cc3d647cfdeebe9e6.1564603467.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] trace2: trim whitespace in start message in perf target format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-01T21:34:37Z","receivedAt":"2019-08-01T21:34:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jeff Hostetler via GitGitGadget\" writes:\n\n> From: Jeff Hostetler \n>\n> Trim leading/trailing whitespace from the command line\n> printed in the \"start\" message in the perf target format.\n>\n> We use `sq_quote_argv_pretty()` to format the message\n> and it adds a leading space to the output. Trim that.\n\nstrbuf_trim() not just drops a single leading space, but removes\nconsecutive spaces from both ends. But the first char after the SP\ncomes from the first arg, and it can never be a whitespace (as a\npayload that begins with a whitespace will be quoted, so it will be\na single quote), and the last char in the buffer would also be\neither a closing single quote (if the last argument ends with a\nwhitespace) or a non whitespace \"safe\" character, so it is safe to\nuse strbuf_trim() here.\n\nI wonder if we want to lose the prepending of SP from\nsq_quote_argv_pretty(), though:\n\n * run-command.c::trace_run_command() does rely on having SP there,\n so the caller needs adjusting if we did so.\n\n * trace.c::trace_argv_vprintf_fl() also needs SP there after the\n caller supplied format.\n\n * trace.c::print_command_performance_atexit() expects command_line\n begins with the extra SP left by the sq_quote_argv_pretty()\n called by the trace_command_performance(); the format string\n given to trace_performance_leave() there needs adjusting.\n\n\nBy the way, use of sq_quote_argv_pretty() in builtin/rebase.c on\nopts->git_am_opts.argv done in run_specific_rebase() is dubious.\nThe argv array is made into a single string that safely uses sq,\nappropriate to feed a shell. But that string is passed as the\n\"value\" parameter to add_var() helper that expects to receive a raw\nvalue (hence it calls sq_quote_buf() on the value), resulting in a\nstring that is doubly quoted. I am not sure if that was intended.\n\nIn any case, the patch itself obviously look correct.\n\n> Signed-off-by: Jeff Hostetler \n> ---\n> trace2/tr2_tgt_perf.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\n> index 4a9d99218b..ed4e708f28 100644\n> --- a/trace2/tr2_tgt_perf.c\n> +++ b/trace2/tr2_tgt_perf.c\n> @@ -185,6 +185,7 @@ static void fn_start_fl(const char *file, int line,\n> \tstruct strbuf buf_payload = STRBUF_INIT;\n> \n> \tsq_quote_argv_pretty(&buf_payload, argv);\n> +\tstrbuf_trim(&buf_payload);\n> \n> \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n> \t\t\t NULL, NULL, &buf_payload);\n"},{"id":"379795","messageId":"20190801225741.GQ43313@google.com","threadId":"51565","inReplyTo":"pull.298.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] trace2: clean up formatting in perf target format","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-01T22:57:41Z","receivedAt":"2019-08-01T22:57:50Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.07.31 13:04, Jeff Hostetler via GitGitGadget wrote:\n> This patch series contains a few column alignment and whitespace fixes for\n> perf target format. This should make it a little easier to read.\n> \n> Jeff Hostetler (3):\n> trace2: cleanup column alignment in perf target format\n> trace2: trim whitespace in start message in perf target format\n> trace2: trim whitespace in region messages in perf target format\n> \n> trace2/tr2_tgt_perf.c | 42 ++++++++++++++++++++++++++++++++----------\n> 1 file changed, 32 insertions(+), 10 deletions(-)\n> \n> \n> base-commit: 026dd738a6e5f1e42ef0f390feacb5ed6acc4ee8\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-298%2Fjeffhostetler%2Fupstream-2230-cleanup-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-298/jeffhostetler/upstream-2230-cleanup-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/298\n> -- \n> gitgitgadget\n\nThe series looks good to me.\n"},{"id":"380095","messageId":"827ed342-1079-8145-ed78-1b955264226e@jeffhostetler.com","threadId":"51565","inReplyTo":"xmqqef24muxe.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] trace2: trim whitespace in start message in perf target format","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2019-08-07T20:25:24Z","receivedAt":"2019-08-07T20:25:28Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 8/1/2019 5:34 PM, Junio C Hamano wrote:\n> \"Jeff Hostetler via GitGitGadget\" writes:\n> \n>> From: Jeff Hostetler \n>>\n>> Trim leading/trailing whitespace from the command line\n>> printed in the \"start\" message in the perf target format.\n>>\n>> We use `sq_quote_argv_pretty()` to format the message\n>> and it adds a leading space to the output. Trim that.\n> \n> strbuf_trim() not just drops a single leading space, but removes\n> consecutive spaces from both ends. But the first char after the SP\n> comes from the first arg, and it can never be a whitespace (as a\n> payload that begins with a whitespace will be quoted, so it will be\n> a single quote), and the last char in the buffer would also be\n> either a closing single quote (if the last argument ends with a\n> whitespace) or a non whitespace \"safe\" character, so it is safe to\n> use strbuf_trim() here.\n> \n> I wonder if we want to lose the prepending of SP from\n> sq_quote_argv_pretty(), though:\n\nI was wondering about that too, but I didn't want to presume\nto know what all of the callers wanted. And didn't want to\nslip in such a change last-minute.\n\nI'll re-roll with a new sq_quote_ function that does not\ninclude the leading whitespace and convert/normalize all\nof my trace2/* uses to call it instead. This eliminates\nthe Trace2 code from the larger conversation (and is a\nlittle more efficient than the strbuf_trim()).\n\nJeff\n"},{"id":"380098","messageId":"xmqqef1whfuj.fsf@gitster-ct.c.googlers.com","threadId":"51565","inReplyTo":"827ed342-1079-8145-ed78-1b955264226e@jeffhostetler.com","subject":"Re: [PATCH 2/3] trace2: trim whitespace in start message in perf target format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-07T20:37:24Z","receivedAt":"2019-08-07T20:37:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> On 8/1/2019 5:34 PM, Junio C Hamano wrote:\n>> \"Jeff Hostetler via GitGitGadget\" writes:\n>>\n>>> From: Jeff Hostetler \n>>>\n>>> Trim leading/trailing whitespace from the command line\n>>> printed in the \"start\" message in the perf target format.\n>>>\n>>> We use `sq_quote_argv_pretty()` to format the message\n>>> and it adds a leading space to the output. Trim that.\n>>\n>> strbuf_trim() not just drops a single leading space, but removes\n>> consecutive spaces from both ends. But the first char after the SP\n>> comes from the first arg, and it can never be a whitespace (as a\n>> payload that begins with a whitespace will be quoted, so it will be\n>> a single quote), and the last char in the buffer would also be\n>> either a closing single quote (if the last argument ends with a\n>> whitespace) or a non whitespace \"safe\" character, so it is safe to\n>> use strbuf_trim() here.\n>>\n>> I wonder if we want to lose the prepending of SP from\n>> sq_quote_argv_pretty(), though:\n>\n> I was wondering about that too, but I didn't want to presume\n> to know what all of the callers wanted. And didn't want to\n> slip in such a change last-minute.\n\nI do not think this topic/discussion is a place to do so, either.\nAnd compensating on the caller's side, like your patch did, is\nperfectly fine.\n\nI was more disturbed by potential double quoting in some codepaths\nwhen I did a cursory audit of users of sq_quote_argv_pretty(), and\nwould think that would be of more importance, though.\n"},{"id":"380130","messageId":"pull.298.v2.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/7] trace2: clean up formatting in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:18:58Z","receivedAt":"2019-08-08T14:19:03Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"V2 of this patch series cleans up some whitespace and column alignment\nissues in the trace2 perf and normal formats. It also removes some dead\ncode.\n\nJeff Hostetler (7):\n trace2: cleanup column alignment in perf target format\n trace2: trim whitespace in region messages in perf target format\n trace2: remove dead code in maybe_add_string_va()\n trace2: trim trailing whitespace in normal format error message\n quote: add sq_quote_argv_pretty_ltrim\n trace2: cleanup whitespace in normal format\n trace2: cleanup whitespace in perf format\n\n quote.c | 11 +++++\n quote.h | 1 +\n t/t0211-trace2-perf.sh | 4 +-\n trace2/tr2_tgt_event.c | 5 ---\n trace2/tr2_tgt_normal.c | 33 +++++++--------\n trace2/tr2_tgt_perf.c | 89 +++++++++++++++++++++++++++--------------\n 6 files changed, 90 insertions(+), 53 deletions(-)\n\n\nbase-commit: 026dd738a6e5f1e42ef0f390feacb5ed6acc4ee8\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-298%2Fjeffhostetler%2Fupstream-2230-cleanup-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-298/jeffhostetler/upstream-2230-cleanup-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/298\n\nRange-diff vs v1:\n\n 1: 38b53da31f = 1: 38b53da31f trace2: cleanup column alignment in perf target format\n 2: 834ee95cd2 < -: ---------- trace2: trim whitespace in start message in perf target format\n 3: d2f7cf3908 < -: ---------- trace2: trim whitespace in region messages in perf target format\n -: ---------- > 2: db35099712 trace2: trim whitespace in region messages in perf target format\n -: ---------- > 3: 09dff12a7d trace2: remove dead code in maybe_add_string_va()\n -: ---------- > 4: d5b5a70c1b trace2: trim trailing whitespace in normal format error message\n -: ---------- > 5: 5059776248 quote: add sq_quote_argv_pretty_ltrim\n -: ---------- > 6: bb64bb2513 trace2: cleanup whitespace in normal format\n -: ---------- > 7: a6e5e7c70b trace2: cleanup whitespace in perf format\n\n-- \ngitgitgadget\n"},{"id":"380131","messageId":"38b53da31f174892b564564e0a380c0a154c8f88.1565273938.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 1/7] trace2: cleanup column alignment in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:18:59Z","receivedAt":"2019-08-08T14:19:04Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTruncate/elide very long \"filename:linenumber\" field.\nTruncate region and data \"category\" field if necessary.\nAdjust overall column widths.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_perf.c | 39 +++++++++++++++++++++++++++++----------\n 1 file changed, 29 insertions(+), 10 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex ea0cbbe13e..4a9d99218b 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -21,10 +21,10 @@ static struct tr2_dst tr2dst_perf = { TR2_SYSENV_PERF, 0, 0, 0 };\n */\n static int tr2env_perf_be_brief;\n \n-#define TR2FMT_PERF_FL_WIDTH (50)\n+#define TR2FMT_PERF_FL_WIDTH (28)\n #define TR2FMT_PERF_MAX_EVENT_NAME (12)\n-#define TR2FMT_PERF_REPO_WIDTH (4)\n-#define TR2FMT_PERF_CATEGORY_WIDTH (10)\n+#define TR2FMT_PERF_REPO_WIDTH (3)\n+#define TR2FMT_PERF_CATEGORY_WIDTH (12)\n \n #define TR2_DOTS_BUFFER_SIZE (100)\n #define TR2_INDENT (2)\n@@ -79,17 +79,36 @@ static void perf_fmt_prepare(const char *event_name,\n \n \tif (!tr2env_perf_be_brief) {\n \t\tstruct tr2_tbuf tb_now;\n+\t\tsize_t fl_end_col;\n \n \t\ttr2_tbuf_local_time(&tb_now);\n \t\tstrbuf_addstr(buf, tb_now.buf);\n \t\tstrbuf_addch(buf, ' ');\n \n-\t\tif (file && *file)\n-\t\t\tstrbuf_addf(buf, \"%s:%d \", file, line);\n-\t\twhile (buf->len < TR2FMT_PERF_FL_WIDTH)\n+\t\tfl_end_col = buf->len + TR2FMT_PERF_FL_WIDTH;\n+\n+\t\tif (file && *file) {\n+\t\t\tstruct strbuf buf_fl = STRBUF_INIT;\n+\n+\t\t\tstrbuf_addf(&buf_fl, \"%s:%d\", file, line);\n+\n+\t\t\tif (buf_fl.len <= TR2FMT_PERF_FL_WIDTH)\n+\t\t\t\tstrbuf_addbuf(buf, &buf_fl);\n+\t\t\telse {\n+\t\t\t\tsize_t avail = TR2FMT_PERF_FL_WIDTH - 3;\n+\t\t\t\tstrbuf_addstr(buf, \"...\");\n+\t\t\t\tstrbuf_add(buf,\n+\t\t\t\t\t &buf_fl.buf[buf_fl.len - avail],\n+\t\t\t\t\t avail);\n+\t\t\t}\n+\n+\t\t\tstrbuf_release(&buf_fl);\n+\t\t}\n+\n+\t\twhile (buf->len < fl_end_col)\n \t\t\tstrbuf_addch(buf, ' ');\n \n-\t\tstrbuf_addstr(buf, \"| \");\n+\t\tstrbuf_addstr(buf, \" | \");\n \t}\n \n \tstrbuf_addf(buf, \"d%d | \", tr2_sid_depth());\n@@ -102,7 +121,7 @@ static void perf_fmt_prepare(const char *event_name,\n \t\tstrbuf_addf(buf, \"r%d \", repo->trace2_repo_id);\n \twhile (buf->len < len)\n \t\tstrbuf_addch(buf, ' ');\n-\tstrbuf_addstr(buf, \"| \");\n+\tstrbuf_addstr(buf, \" | \");\n \n \tif (p_us_elapsed_absolute)\n \t\tstrbuf_addf(buf, \"%9.6f | \",\n@@ -116,8 +135,8 @@ static void perf_fmt_prepare(const char *event_name,\n \telse\n \t\tstrbuf_addf(buf, \"%9s | \", \" \");\n \n-\tstrbuf_addf(buf, \"%-*s | \", TR2FMT_PERF_CATEGORY_WIDTH,\n-\t\t (category ? category : \"\"));\n+\tstrbuf_addf(buf, \"%-*.*s | \", TR2FMT_PERF_CATEGORY_WIDTH,\n+\t\t TR2FMT_PERF_CATEGORY_WIDTH, (category ? category : \"\"));\n \n \tif (ctx->nr_open_regions > 0) {\n \t\tint len_indent = TR2_INDENT_LENGTH(ctx);\n-- \ngitgitgadget\n\n"},{"id":"380132","messageId":"09dff12a7dfe2219feab85b3fe611aa5c4196375.1565273938.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 3/7] trace2: remove dead code in maybe_add_string_va()","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:19:01Z","receivedAt":"2019-08-08T14:19:06Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nRemove an unnecessary \"if\" block in maybe_add_string_va().\n\nCommit \"ad006fe419e trace2: NULL is not allowed for va_list\"\nchanged \"if (fmt && *fmt && ap)\" to just \"if (fmt && *fmt)\"\nbecause it isn't safe to treat 'ap' as a pointer. This made\nthe \"if\" block following it unnecessary.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_event.c | 5 -----\n trace2/tr2_tgt_normal.c | 5 -----\n trace2/tr2_tgt_perf.c | 5 -----\n 3 files changed, 15 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c\nindex c2852d1bd2..9bcac20d1b 100644\n--- a/trace2/tr2_tgt_event.c\n+++ b/trace2/tr2_tgt_event.c\n@@ -205,11 +205,6 @@ static void maybe_add_string_va(struct json_writer *jw, const char *field_name,\n \t\tstrbuf_release(&buf);\n \t\treturn;\n \t}\n-\n-\tif (fmt && *fmt) {\n-\t\tjw_object_string(jw, field_name, fmt);\n-\t\treturn;\n-\t}\n }\n \n static void fn_error_va_fl(const char *file, int line, const char *fmt,\ndiff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\nindex 00b116d797..47a1882557 100644\n--- a/trace2/tr2_tgt_normal.c\n+++ b/trace2/tr2_tgt_normal.c\n@@ -135,11 +135,6 @@ static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n \t\tva_end(copy_ap);\n \t\treturn;\n \t}\n-\n-\tif (fmt && *fmt) {\n-\t\tstrbuf_addstr(buf, fmt);\n-\t\treturn;\n-\t}\n }\n \n static void fn_error_va_fl(const char *file, int line, const char *fmt,\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex fb845cb627..8e52395de2 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -239,11 +239,6 @@ static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n \t\tva_end(copy_ap);\n \t\treturn;\n \t}\n-\n-\tif (fmt && *fmt) {\n-\t\tstrbuf_addstr(buf, fmt);\n-\t\treturn;\n-\t}\n }\n \n static void fn_error_va_fl(const char *file, int line, const char *fmt,\n-- \ngitgitgadget\n\n"},{"id":"380133","messageId":"bb64bb251331ab60c2feac365bee9cbc7f6c16d1.1565273938.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 6/7] trace2: cleanup whitespace in normal format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:19:03Z","receivedAt":"2019-08-08T14:19:08Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nMake use of new sq_quote_argv_pretty_ltrim() to normalize\nhow we handle leading whitespace in normal format messages.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_normal.c | 21 ++++++++++++---------\n 1 file changed, 12 insertions(+), 9 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\nindex 213724d5cb..5de2f8214c 100644\n--- a/trace2/tr2_tgt_normal.c\n+++ b/trace2/tr2_tgt_normal.c\n@@ -87,7 +87,7 @@ static void fn_start_fl(const char *file, int line,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tstrbuf_addstr(&buf_payload, \"start \");\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n@@ -186,8 +186,8 @@ static void fn_alias_fl(const char *file, int line, const char *alias,\n {\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addf(&buf_payload, \"alias %s ->\", alias);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addf(&buf_payload, \"alias %s -> \", alias);\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n@@ -198,12 +198,12 @@ static void fn_child_start_fl(const char *file, int line,\n {\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addf(&buf_payload, \"child_start[%d] \", cmd->trace2_child_id);\n+\tstrbuf_addf(&buf_payload, \"child_start[%d]\", cmd->trace2_child_id);\n \n \tif (cmd->dir) {\n \t\tstrbuf_addstr(&buf_payload, \" cd\");\n \t\tsq_quote_buf_pretty(&buf_payload, cmd->dir);\n-\t\tstrbuf_addstr(&buf_payload, \"; \");\n+\t\tstrbuf_addstr(&buf_payload, \";\");\n \t}\n \n \t/*\n@@ -211,9 +211,10 @@ static void fn_child_start_fl(const char *file, int line,\n \t * See trace_add_env() in run-command.c as used by original trace.c\n \t */\n \n+\tstrbuf_addch(&buf_payload, ' ');\n \tif (cmd->git_cmd)\n-\t\tstrbuf_addstr(&buf_payload, \"git\");\n-\tsq_quote_argv_pretty(&buf_payload, cmd->argv);\n+\t\tstrbuf_addstr(&buf_payload, \"git \");\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, cmd->argv);\n \n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n@@ -238,9 +239,11 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tstrbuf_addf(&buf_payload, \"exec[%d] \", exec_id);\n-\tif (exe)\n+\tif (exe) {\n \t\tstrbuf_addstr(&buf_payload, exe);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\t\tstrbuf_addch(&buf_payload, ' ');\n+\t}\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n-- \ngitgitgadget\n\n"},{"id":"380134","messageId":"5059776248b6686faaff37c97aa63d0212579cd8.1565273938.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:19:02Z","receivedAt":"2019-08-08T14:19:08Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nCreate version of sq_quote_argv_pretty() that does not\ninsert a leading space before argv[0].\n\nSigned-off-by: Jeff Hostetler \n---\n quote.c | 11 +++++++++++\n quote.h | 1 +\n 2 files changed, 12 insertions(+)\n\ndiff --git a/quote.c b/quote.c\nindex 7f2aa6faa4..7cad8798ac 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -94,6 +94,17 @@ void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n \t}\n }\n \n+void sq_quote_argv_pretty_ltrim(struct strbuf *dst, const char **argv)\n+{\n+\tint i;\n+\n+\tfor (i = 0; argv[i]; i++) {\n+\t\tif (i > 0)\n+\t\t\tstrbuf_addch(dst, ' ');\n+\t\tsq_quote_buf_pretty(dst, argv[i]);\n+\t}\n+}\n+\n static char *sq_dequote_step(char *arg, char **next)\n {\n \tchar *dst = arg;\ndiff --git a/quote.h b/quote.h\nindex fb08dc085c..3b3d041a61 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -40,6 +40,7 @@ void sq_quotef(struct strbuf *, const char *fmt, ...);\n */\n void sq_quote_buf_pretty(struct strbuf *, const char *src);\n void sq_quote_argv_pretty(struct strbuf *, const char **argv);\n+void sq_quote_argv_pretty_ltrim(struct strbuf *, const char **argv);\n \n /* This unwraps what sq_quote() produces in place, but returns\n * NULL if the input does not look like what sq_quote would have\n-- \ngitgitgadget\n\n"},{"id":"380135","messageId":"a6e5e7c70b1053fb6c232d01bf00db96585334aa.1565273938.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 7/7] trace2: cleanup whitespace in perf format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:19:04Z","receivedAt":"2019-08-08T14:19:10Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nMake use of new sq_quote_argv_pretty_ltrim() to normalize\nhow we handle leading whitespace in perf format messages.\n\nSigned-off-by: Jeff Hostetler \n---\n t/t0211-trace2-perf.sh | 4 ++--\n trace2/tr2_tgt_perf.c | 31 ++++++++++++++++++++-----------\n 2 files changed, 22 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t0211-trace2-perf.sh b/t/t0211-trace2-perf.sh\nindex 2c3ad6e8c1..6ee8ee3b67 100755\n--- a/t/t0211-trace2-perf.sh\n+++ b/t/t0211-trace2-perf.sh\n@@ -130,11 +130,11 @@ test_expect_success 'perf stream, child processes' '\n \t\td0|main|version|||||$V\n \t\td0|main|start||_T_ABS_|||_EXE_ trace2 004child test-tool trace2 004child test-tool trace2 001return 0\n \t\td0|main|cmd_name|||||trace2 (trace2)\n-\t\td0|main|child_start||_T_ABS_|||[ch0] class:? argv: test-tool trace2 004child test-tool trace2 001return 0\n+\t\td0|main|child_start||_T_ABS_|||[ch0] class:? argv:[test-tool trace2 004child test-tool trace2 001return 0]\n \t\td1|main|version|||||$V\n \t\td1|main|start||_T_ABS_|||_EXE_ trace2 004child test-tool trace2 001return 0\n \t\td1|main|cmd_name|||||trace2 (trace2/trace2)\n-\t\td1|main|child_start||_T_ABS_|||[ch0] class:? argv: test-tool trace2 001return 0\n+\t\td1|main|child_start||_T_ABS_|||[ch0] class:? argv:[test-tool trace2 001return 0]\n \t\td2|main|version|||||$V\n \t\td2|main|start||_T_ABS_|||_EXE_ trace2 001return 0\n \t\td2|main|cmd_name|||||trace2 (trace2/trace2/trace2)\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex 8e52395de2..59c03a5ed4 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -184,7 +184,7 @@ static void fn_start_fl(const char *file, int line,\n \tconst char *event_name = \"start\";\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n@@ -299,8 +299,9 @@ static void fn_alias_fl(const char *file, int line, const char *alias,\n \tconst char *event_name = \"alias\";\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addf(&buf_payload, \"alias:%s argv:\", alias);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addf(&buf_payload, \"alias:%s argv:[\", alias);\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n+\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL,\n \t\t\t &buf_payload);\n@@ -329,10 +330,14 @@ static void fn_child_start_fl(const char *file, int line,\n \t\tsq_quote_buf_pretty(&buf_payload, cmd->dir);\n \t}\n \n-\tstrbuf_addstr(&buf_payload, \" argv:\");\n-\tif (cmd->git_cmd)\n-\t\tstrbuf_addstr(&buf_payload, \" git\");\n-\tsq_quote_argv_pretty(&buf_payload, cmd->argv);\n+\tstrbuf_addstr(&buf_payload, \" argv:[\");\n+\tif (cmd->git_cmd) {\n+\t\tstrbuf_addstr(&buf_payload, \"git\");\n+\t\tif (cmd->argv[0])\n+\t\t\tstrbuf_addch(&buf_payload, ' ');\n+\t}\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, cmd->argv);\n+\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n@@ -383,10 +388,14 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tstrbuf_addf(&buf_payload, \"id:%d \", exec_id);\n-\tstrbuf_addstr(&buf_payload, \"argv:\");\n-\tif (exe)\n-\t\tstrbuf_addf(&buf_payload, \" %s\", exe);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addstr(&buf_payload, \"argv:[\");\n+\tif (exe) {\n+\t\tstrbuf_addstr(&buf_payload, exe);\n+\t\tif (argv[0])\n+\t\t\tstrbuf_addch(&buf_payload, ' ');\n+\t}\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n+\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n-- \ngitgitgadget\n"},{"id":"380136","messageId":"d5b5a70c1b0012e1fca29cdcbefa9df2155bb34b.1565273938.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 4/7] trace2: trim trailing whitespace in normal format error message","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:19:02Z","receivedAt":"2019-08-08T14:19:11Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nAvoid creating unnecessary trailing whitespace in normal\ntarget format error messages when the message is omitted.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_normal.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\nindex 47a1882557..213724d5cb 100644\n--- a/trace2/tr2_tgt_normal.c\n+++ b/trace2/tr2_tgt_normal.c\n@@ -142,8 +142,11 @@ static void fn_error_va_fl(const char *file, int line, const char *fmt,\n {\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addstr(&buf_payload, \"error \");\n-\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\tstrbuf_addstr(&buf_payload, \"error\");\n+\tif (fmt && *fmt) {\n+\t\tstrbuf_addch(&buf_payload, ' ');\n+\t\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t}\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n-- \ngitgitgadget\n\n"},{"id":"380137","messageId":"db3509971281bbaadf2003c3b3fa3058ca5f9ab0.1565273938.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 2/7] trace2: trim whitespace in region messages in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-08T14:19:00Z","receivedAt":"2019-08-08T14:19:11Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nAvoid unecessary trailing whitespace in \"region_enter\" and \"region_leave\"\nmessages in perf target format.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_perf.c | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex 4a9d99218b..fb845cb627 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -452,8 +452,11 @@ static void fn_region_enter_printf_va_fl(const char *file, int line,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tif (label)\n-\t\tstrbuf_addf(&buf_payload, \"label:%s \", label);\n-\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t\tstrbuf_addf(&buf_payload, \"label:%s\", label);\n+\tif (fmt && *fmt) {\n+\t\tstrbuf_addch(&buf_payload, ' ');\n+\t\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t}\n \n \tperf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,\n \t\t\t NULL, category, &buf_payload);\n@@ -469,8 +472,11 @@ static void fn_region_leave_printf_va_fl(\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tif (label)\n-\t\tstrbuf_addf(&buf_payload, \"label:%s \", label);\n-\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t\tstrbuf_addf(&buf_payload, \"label:%s\", label);\n+\tif (fmt && *fmt) {\n+\t\tstrbuf_addch(&buf_payload, ' ' );\n+\t\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t}\n \n \tperf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,\n \t\t\t &us_elapsed_region, category, &buf_payload);\n-- \ngitgitgadget\n\n"},{"id":"380150","messageId":"xmqqh86rfs70.fsf@gitster-ct.c.googlers.com","threadId":"51565","inReplyTo":"5059776248b6686faaff37c97aa63d0212579cd8.1565273938.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-08T18:05:55Z","receivedAt":"2019-08-08T18:06:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jeff Hostetler via GitGitGadget\" writes:\n\n> From: Jeff Hostetler \n>\n> Create version of sq_quote_argv_pretty() that does not\n> insert a leading space before argv[0].\n>\n> Signed-off-by: Jeff Hostetler \n> ---\n> quote.c | 11 +++++++++++\n> quote.h | 1 +\n> 2 files changed, 12 insertions(+)\n\nI am OK with the basic idea, but I am somewhat unhappy about this\nparticular patch for two reasons:\n\n - If we were to keep this as a part of proper API in the longer\n term, the current sq_quote_argv_pretty() should be rewritten to\n use this to avoid repetition (e.g. as long as !!*argv, add a SP\n and then call this new thing);\n\n - something_ltrim() sounds as if you munge what is passed to you\n and chop off the left end, but that is not what this does.\n\nNow, what is the right name for this new thing? What does it do?\n\nIt looks to me that it appends each element of argv[], quoting it as\nneeded, and with SP in between. So the right name for the family of\nthese functions should be around \"append\", which is the primary thing\nthey do, with \"quoted\" somewhere.\n\nHaving made the primary purpose of the helper clearer leads me to\nwonder if \"do not add SP before the first element, i.e. argv[0]\", is\nreally what we want. If we always clear the *dst strbuf before\nstarting to serialize argv[] into it, then the behaviour would make\nsense, but we do not---we are \"appending\".\n\nAs long as we are appending, would we be better off doing something\nsillily magical like this instead, I have to wonder?\n\n\tvoid sq_append_strings_quoted(struct strbuf *buf, const char **av)\n\t{\n\t\tint i;\n\n\t\tfor (i = 0; av[i]; i++) {\n\t\t\tif (buf->len)\n\t\t\t\tstrbuf_addch(buf, ' ');\n\t\t\tsq_quote_buf_pretty(buf, argv[0]);\n\t\t}\n\t}\n\nThat is, \"if we are appending to an existing string, have SP to\nseparate the first element from that existing string; treat the\nremaining elements the same way (if the buffer is empty, there is no\npoint adding SP at the beginning)\".\n\nI may have found a long-standing bug in sq_quote_buf_pretty(), by\nthe way. What does it produce when *src is an empty string of\nlength 0? It does not add anything to dst, but shouldn't we be\nadding two single-quotes (i.e. an empty string inside sq pair)?\n\n> diff --git a/quote.c b/quote.c\n> index 7f2aa6faa4..7cad8798ac 100644\n> --- a/quote.c\n> +++ b/quote.c\n> @@ -94,6 +94,17 @@ void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n> \t}\n> }\n> \n> +void sq_quote_argv_pretty_ltrim(struct strbuf *dst, const char **argv)\n> +{\n> +\tint i;\n> +\n> +\tfor (i = 0; argv[i]; i++) {\n> +\t\tif (i > 0)\n> +\t\t\tstrbuf_addch(dst, ' ');\n> +\t\tsq_quote_buf_pretty(dst, argv[i]);\n> +\t}\n> +}\n> +\n> static char *sq_dequote_step(char *arg, char **next)\n> {\n> \tchar *dst = arg;\n> diff --git a/quote.h b/quote.h\n> index fb08dc085c..3b3d041a61 100644\n> --- a/quote.h\n> +++ b/quote.h\n> @@ -40,6 +40,7 @@ void sq_quotef(struct strbuf *, const char *fmt, ...);\n> */\n> void sq_quote_buf_pretty(struct strbuf *, const char *src);\n> void sq_quote_argv_pretty(struct strbuf *, const char **argv);\n> +void sq_quote_argv_pretty_ltrim(struct strbuf *, const char **argv);\n> \n> /* This unwraps what sq_quote() produces in place, but returns\n> * NULL if the input does not look like what sq_quote would have\n"},{"id":"380157","messageId":"7dbee619-2495-6426-b02b-49fd59e4f028@jeffhostetler.com","threadId":"51565","inReplyTo":"xmqqh86rfs70.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2019-08-08T19:04:24Z","receivedAt":"2019-08-08T19:04:28Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 8/8/2019 2:05 PM, Junio C Hamano wrote:\n> \"Jeff Hostetler via GitGitGadget\" writes:\n> \n>> From: Jeff Hostetler \n>>\n>> Create version of sq_quote_argv_pretty() that does not\n>> insert a leading space before argv[0].\n>>\n>> Signed-off-by: Jeff Hostetler \n>> ---\n>> quote.c | 11 +++++++++++\n>> quote.h | 1 +\n>> 2 files changed, 12 insertions(+)\n> \n> I am OK with the basic idea, but I am somewhat unhappy about this\n> particular patch for two reasons:\n> \n> - If we were to keep this as a part of proper API in the longer\n> term, the current sq_quote_argv_pretty() should be rewritten to\n> use this to avoid repetition (e.g. as long as !!*argv, add a SP\n> and then call this new thing);\n> \n> - something_ltrim() sounds as if you munge what is passed to you\n> and chop off the left end, but that is not what this does.\n> \n> Now, what is the right name for this new thing? What does it do?\n\nI struggled with the proper name for this.\nAnd even thought about adding a 3rd arg to the current function to\nindicate whether to have the leading SP before argv[0], but wasn't\nsure if that was too disruptive.\n\n> \n> It looks to me that it appends each element of argv[], quoting it as\n> needed, and with SP in between. So the right name for the family of\n> these functions should be around \"append\", which is the primary thing\n> they do, with \"quoted\" somewhere.\n> \n> Having made the primary purpose of the helper clearer leads me to\n> wonder if \"do not add SP before the first element, i.e. argv[0]\", is\n> really what we want. If we always clear the *dst strbuf before\n> starting to serialize argv[] into it, then the behaviour would make\n> sense, but we do not---we are \"appending\".\n> \n> As long as we are appending, would we be better off doing something\n> sillily magical like this instead, I have to wonder?\n> \n> \tvoid sq_append_strings_quoted(struct strbuf *buf, const char **av)\n> \t{\n> \t\tint i;\n> \n> \t\tfor (i = 0; av[i]; i++) {\n> \t\t\tif (buf->len)\n> \t\t\t\tstrbuf_addch(buf, ' ');\n> \t\t\tsq_quote_buf_pretty(buf, argv[0]);\n> \t\t}\n> \t}\n> \n> That is, \"if we are appending to an existing string, have SP to\n> separate the first element from that existing string; treat the\n> remaining elements the same way (if the buffer is empty, there is no\n> point adding SP at the beginning)\".\n\nI don't think that would do what we want. We don't know what the\ncaller's expectations are. In my uses in commits 6/7 and 7/7 I\nalready added the leading chars I wanted in the strbuf before calling\nsq_quote_argv_pretty_ltrim() and assumed the output would be a true\nappend. For example:\n\n+\tstrbuf_addf(&buf_payload, \"alias:%s argv:[\", alias);\n+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n+\tstrbuf_addch(&buf_payload, ']');\n\nI like your suggestion of putting my new function in the _append_\ncategory. I think I'll add the 3rd arg to this and then it will\nbe completely specified and I can get rid of the _ltrim suffix.\n\nI'll re-roll this.\n\n> \n> I may have found a long-standing bug in sq_quote_buf_pretty(), by\n> the way. What does it produce when *src is an empty string of\n> length 0? It does not add anything to dst, but shouldn't we be\n> adding two single-quotes (i.e. an empty string inside sq pair)?\n\nI would think so. I did a quick grep and most of the calls looked\nguarded, so I don't think this is urgent. I'll address this in a\nseparate commit shortly.\n\nThanks\nJeff\n\n\n> \n>> diff --git a/quote.c b/quote.c\n>> index 7f2aa6faa4..7cad8798ac 100644\n>> --- a/quote.c\n>> +++ b/quote.c\n>> @@ -94,6 +94,17 @@ void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n>> \t}\n>> }\n>> \n>> +void sq_quote_argv_pretty_ltrim(struct strbuf *dst, const char **argv)\n>> +{\n>> +\tint i;\n>> +\n>> +\tfor (i = 0; argv[i]; i++) {\n>> +\t\tif (i > 0)\n>> +\t\t\tstrbuf_addch(dst, ' ');\n>> +\t\tsq_quote_buf_pretty(dst, argv[i]);\n>> +\t}\n>> +}\n>> +\n>> static char *sq_dequote_step(char *arg, char **next)\n>> {\n>> \tchar *dst = arg;\n>> diff --git a/quote.h b/quote.h\n>> index fb08dc085c..3b3d041a61 100644\n>> --- a/quote.h\n>> +++ b/quote.h\n>> @@ -40,6 +40,7 @@ void sq_quotef(struct strbuf *, const char *fmt, ...);\n>> */\n>> void sq_quote_buf_pretty(struct strbuf *, const char *src);\n>> void sq_quote_argv_pretty(struct strbuf *, const char **argv);\n>> +void sq_quote_argv_pretty_ltrim(struct strbuf *, const char **argv);\n>> \n>> /* This unwraps what sq_quote() produces in place, but returns\n>> * NULL if the input does not look like what sq_quote would have\n"},{"id":"380163","messageId":"xmqqzhkje8ao.fsf@gitster-ct.c.googlers.com","threadId":"51565","inReplyTo":"7dbee619-2495-6426-b02b-49fd59e4f028@jeffhostetler.com","subject":"Re: [PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-08T20:01:03Z","receivedAt":"2019-08-08T20:01:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n>> That is, \"if we are appending to an existing string, have SP to\n>> separate the first element from that existing string; treat the\n>> remaining elements the same way (if the buffer is empty, there is no\n>> point adding SP at the beginning)\".\n>\n> I don't think that would do what we want.\n\nI know that there are current callers of quote_argv_pretty that\neither (1) expects that it will always get the leading SP for free,\nor (2) has to work the unwanted SP around (basically, the places you\nchanged from quote_argv_pretty to quote_argv_pretty_ltrim in your\nseries). But I wanted to see if we can come up with a single helper\nwhose behaviour is easy to explain and understand that both existing\nand new callers can adopt---and if the resulting codebase becomes\neasy to understand and maintain overall. And if that would give us\nthe ideal longer term direction.\n\n>> I may have found a long-standing bug in sq_quote_buf_pretty(), by\n>> the way. What does it produce when *src is an empty string of\n>> length 0? It does not add anything to dst, but shouldn't we be\n>> adding two single-quotes (i.e. an empty string inside sq pair)?\n>\n> I would think so. I did a quick grep and most of the calls looked\n> guarded, so I don't think this is urgent. I'll address this in a\n> separate commit shortly.\n\nThanks.\n"},{"id":"380181","messageId":"60c7b26c-e598-182b-421e-bdf5c0a8ac9f@web.de","threadId":"51565","inReplyTo":"7dbee619-2495-6426-b02b-49fd59e4f028@jeffhostetler.com","subject":"Re: [PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-08T22:49:50Z","receivedAt":"2019-08-08T22:50:11Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 08.08.19 um 21:04 schrieb Jeff Hostetler:\n> On 8/8/2019 2:05 PM, Junio C Hamano wrote:\n>> Having made the primary purpose of the helper clearer leads me to\n>> wonder if \"do not add SP before the first element, i.e. argv[0]\", is\n>> really what we want.  If we always clear the *dst strbuf before\n>> starting to serialize argv[] into it, then the behaviour would make\n>> sense, but we do not---we are \"appending\".\n>>\n>> As long as we are appending, would we be better off doing something\n>> sillily magical like this instead, I have to wonder?\n>>\n>>     void sq_append_strings_quoted(struct strbuf *buf, const char **av)\n>>     {\n>>         int i;\n>>\n>>         for (i = 0; av[i]; i++) {\n>>             if (buf->len)\n>>                 strbuf_addch(buf, ' ');\n>>             sq_quote_buf_pretty(buf, argv[0]);\n>>         }\n>>     }\n>>\n>> That is, \"if we are appending to an existing string, have SP to\n>> separate the first element from that existing string; treat the\n>> remaining elements the same way (if the buffer is empty, there is no\n>> point adding SP at the beginning)\".\n>\n> I don't think that would do what we want.  We don't know what the\n> caller's expectations are.  In my uses in commits 6/7 and 7/7 I\n> already added the leading chars I wanted in the strbuf before calling\n> sq_quote_argv_pretty_ltrim() and assumed the output would be a true\n> append.  For example:\n>\n> +    strbuf_addf(&buf_payload, \"alias:%s argv:[\", alias);\n> +    sq_quote_argv_pretty_ltrim(&buf_payload, argv);\n> +    strbuf_addch(&buf_payload, ']');\n>\n> I like your suggestion of putting my new function in the _append_\n> category.  I think I'll add the 3rd arg to this and then it will\n> be completely specified and I can get rid of the _ltrim suffix.\n\nTwo observations:\n\nIf callers want to add something before a joined delimited list, they\nalready can with a strbuf_add* call. No need to add that feature to\na function that joins lists.\n\nAnd repetitions of repetitions (loops) are boring.\n\nApologies in advance for any coffee stains on your monitor, but\nhere's how I would start, probably followed by attempts to inline the\nfunctions that become trivial wrappers:\n\n---\n quote.c | 18 ++++--------------\n strbuf.c | 20 +++++++++++++-------\n strbuf.h | 8 ++++++++\n 3 files changed, 25 insertions(+), 21 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex 7f2aa6faa4..f422188852 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -74,24 +74,14 @@ void sq_quotef(struct strbuf *dst, const char *fmt, ...)\n\n void sq_quote_argv(struct strbuf *dst, const char **argv)\n {\n-\tint i;\n-\n-\t/* Copy into destination buffer. */\n-\tstrbuf_grow(dst, 255);\n-\tfor (i = 0; argv[i]; ++i) {\n-\t\tstrbuf_addch(dst, ' ');\n-\t\tsq_quote_buf(dst, argv[i]);\n-\t}\n+\tstrbuf_addch(dst, ' ');\n+\tstrbuf_map_join_argv(dst, argv, sq_quote_buf, \" \");\n }\n\n void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n {\n-\tint i;\n-\n-\tfor (i = 0; argv[i]; i++) {\n-\t\tstrbuf_addch(dst, ' ');\n-\t\tsq_quote_buf_pretty(dst, argv[i]);\n-\t}\n+\tstrbuf_addch(dst, ' ');\n+\tstrbuf_map_join_argv(dst, argv, sq_quote_buf_pretty, \" \");\n }\n\n static char *sq_dequote_step(char *arg, char **next)\ndiff --git a/strbuf.c b/strbuf.c\nindex d30f916858..d337853b53 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -304,17 +304,23 @@ void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2)\n \tstrbuf_setlen(sb, sb->len + sb2->len);\n }\n\n+void strbuf_map_join_argv(struct strbuf *sb, const char **argv,\n+\t\t\t void (*fn)(struct strbuf *, const char *),\n+\t\t\t const char *separator)\n+{\n+\twhile (*argv) {\n+\t\tfn(sb, *argv++);\n+\t\tif (*argv)\n+\t\t\tstrbuf_addstr(sb, separator);\n+\t}\n+}\n+\n const char *strbuf_join_argv(struct strbuf *buf,\n \t\t\t int argc, const char **argv, char delim)\n {\n-\tif (!argc)\n-\t\treturn buf->buf;\n+\tchar separator[] = { delim, '\\0' };\n\n-\tstrbuf_addstr(buf, *argv);\n-\twhile (--argc) {\n-\t\tstrbuf_addch(buf, delim);\n-\t\tstrbuf_addstr(buf, *(++argv));\n-\t}\n+\tstrbuf_map_join_argv(buf, argv, strbuf_addstr, separator);\n\n \treturn buf->buf;\n }\ndiff --git a/strbuf.h b/strbuf.h\nindex f62278a0be..7adeff94a7 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -297,6 +297,14 @@ static inline void strbuf_addstr(struct strbuf *sb, const char *s)\n */\n void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2);\n\n+/**\n+ * Apply `fn` to `sb` and each element of the NULL-terminated array\n+ * `argv`. Add `separator` between these invocations.\n+ */\n+void strbuf_map_join_argv(struct strbuf *sb, const char **argv,\n+\t\t\t void (*fn)(struct strbuf *, const char *),\n+\t\t\t const char *separator);\n+\n /**\n * Join the arguments into a buffer. `delim` is put between every\n * two arguments.\n--\n2.22.0\n"},{"id":"380213","messageId":"pull.298.v3.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v3 0/7] trace2: clean up formatting in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:52Z","receivedAt":"2019-08-09T15:00:56Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"V2 of this patch series cleans up some whitespace and column alignment\nissues in the trace2 perf and normal formats. It also removes some dead\ncode.\n\nJeff Hostetler (7):\n trace2: cleanup column alignment in perf target format\n trace2: trim whitespace in region messages in perf target format\n trace2: remove dead code in maybe_add_string_va()\n trace2: trim trailing whitespace in normal format error message\n quote: add sq_append_quote_argv_pretty()\n trace2: cleanup whitespace in normal format\n trace2: cleanup whitespace in perf format\n\n quote.c | 18 ++++++++-\n quote.h | 1 +\n t/t0211-trace2-perf.sh | 4 +-\n trace2/tr2_tgt_event.c | 5 ---\n trace2/tr2_tgt_normal.c | 35 ++++++++--------\n trace2/tr2_tgt_perf.c | 89 +++++++++++++++++++++++++++--------------\n 6 files changed, 97 insertions(+), 55 deletions(-)\n\n\nbase-commit: 026dd738a6e5f1e42ef0f390feacb5ed6acc4ee8\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-298%2Fjeffhostetler%2Fupstream-2230-cleanup-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-298/jeffhostetler/upstream-2230-cleanup-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/298\n\nRange-diff vs v2:\n\n 1: 38b53da31f = 1: 38b53da31f trace2: cleanup column alignment in perf target format\n 2: db35099712 = 2: db35099712 trace2: trim whitespace in region messages in perf target format\n 3: 09dff12a7d = 3: 09dff12a7d trace2: remove dead code in maybe_add_string_va()\n 4: d5b5a70c1b = 4: d5b5a70c1b trace2: trim trailing whitespace in normal format error message\n 5: 5059776248 < -: ---------- quote: add sq_quote_argv_pretty_ltrim\n -: ---------- > 5: 2002108a30 quote: add sq_append_quote_argv_pretty()\n 6: bb64bb2513 ! 6: d02a0608d7 trace2: cleanup whitespace in normal format\n @@ -2,7 +2,7 @@\n \n trace2: cleanup whitespace in normal format\n \n - Make use of new sq_quote_argv_pretty_ltrim() to normalize\n + Make use of new sq_append_quote_argv_pretty() to normalize\n how we handle leading whitespace in normal format messages.\n \n Signed-off-by: Jeff Hostetler \n @@ -15,7 +15,7 @@\n \n \tstrbuf_addstr(&buf_payload, \"start \");\n -\tsq_quote_argv_pretty(&buf_payload, argv);\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n @@ -26,7 +26,7 @@\n -\tstrbuf_addf(&buf_payload, \"alias %s ->\", alias);\n -\tsq_quote_argv_pretty(&buf_payload, argv);\n +\tstrbuf_addf(&buf_payload, \"alias %s -> \", alias);\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n @@ -38,7 +38,8 @@\n +\tstrbuf_addf(&buf_payload, \"child_start[%d]\", cmd->trace2_child_id);\n \n \tif (cmd->dir) {\n - \t\tstrbuf_addstr(&buf_payload, \" cd\");\n +-\t\tstrbuf_addstr(&buf_payload, \" cd\");\n ++\t\tstrbuf_addstr(&buf_payload, \" cd \");\n \t\tsq_quote_buf_pretty(&buf_payload, cmd->dir);\n -\t\tstrbuf_addstr(&buf_payload, \"; \");\n +\t\tstrbuf_addstr(&buf_payload, \";\");\n @@ -54,7 +55,7 @@\n -\t\tstrbuf_addstr(&buf_payload, \"git\");\n -\tsq_quote_argv_pretty(&buf_payload, cmd->argv);\n +\t\tstrbuf_addstr(&buf_payload, \"git \");\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, cmd->argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, cmd->argv);\n \n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n @@ -68,7 +69,7 @@\n -\tsq_quote_argv_pretty(&buf_payload, argv);\n +\t\tstrbuf_addch(&buf_payload, ' ');\n +\t}\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n 7: a6e5e7c70b ! 7: 9ab7393985 trace2: cleanup whitespace in perf format\n @@ -2,7 +2,7 @@\n \n trace2: cleanup whitespace in perf format\n \n - Make use of new sq_quote_argv_pretty_ltrim() to normalize\n + Make use of new sq_append_quote_argv_pretty() to normalize\n how we handle leading whitespace in perf format messages.\n \n Signed-off-by: Jeff Hostetler \n @@ -33,7 +33,7 @@\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n -\tsq_quote_argv_pretty(&buf_payload, argv);\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n @@ -44,7 +44,7 @@\n -\tstrbuf_addf(&buf_payload, \"alias:%s argv:\", alias);\n -\tsq_quote_argv_pretty(&buf_payload, argv);\n +\tstrbuf_addf(&buf_payload, \"alias:%s argv:[\", alias);\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, argv);\n +\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL,\n @@ -63,7 +63,7 @@\n +\t\tif (cmd->argv[0])\n +\t\t\tstrbuf_addch(&buf_payload, ' ');\n +\t}\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, cmd->argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, cmd->argv);\n +\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n @@ -82,7 +82,7 @@\n +\t\tif (argv[0])\n +\t\t\tstrbuf_addch(&buf_payload, ' ');\n +\t}\n -+\tsq_quote_argv_pretty_ltrim(&buf_payload, argv);\n ++\tsq_append_quote_argv_pretty(&buf_payload, argv);\n +\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n\n-- \ngitgitgadget\n"},{"id":"380214","messageId":"38b53da31f174892b564564e0a380c0a154c8f88.1565362851.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 1/7] trace2: cleanup column alignment in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:53Z","receivedAt":"2019-08-09T15:00:57Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTruncate/elide very long \"filename:linenumber\" field.\nTruncate region and data \"category\" field if necessary.\nAdjust overall column widths.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_perf.c | 39 +++++++++++++++++++++++++++++----------\n 1 file changed, 29 insertions(+), 10 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex ea0cbbe13e..4a9d99218b 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -21,10 +21,10 @@ static struct tr2_dst tr2dst_perf = { TR2_SYSENV_PERF, 0, 0, 0 };\n */\n static int tr2env_perf_be_brief;\n \n-#define TR2FMT_PERF_FL_WIDTH (50)\n+#define TR2FMT_PERF_FL_WIDTH (28)\n #define TR2FMT_PERF_MAX_EVENT_NAME (12)\n-#define TR2FMT_PERF_REPO_WIDTH (4)\n-#define TR2FMT_PERF_CATEGORY_WIDTH (10)\n+#define TR2FMT_PERF_REPO_WIDTH (3)\n+#define TR2FMT_PERF_CATEGORY_WIDTH (12)\n \n #define TR2_DOTS_BUFFER_SIZE (100)\n #define TR2_INDENT (2)\n@@ -79,17 +79,36 @@ static void perf_fmt_prepare(const char *event_name,\n \n \tif (!tr2env_perf_be_brief) {\n \t\tstruct tr2_tbuf tb_now;\n+\t\tsize_t fl_end_col;\n \n \t\ttr2_tbuf_local_time(&tb_now);\n \t\tstrbuf_addstr(buf, tb_now.buf);\n \t\tstrbuf_addch(buf, ' ');\n \n-\t\tif (file && *file)\n-\t\t\tstrbuf_addf(buf, \"%s:%d \", file, line);\n-\t\twhile (buf->len < TR2FMT_PERF_FL_WIDTH)\n+\t\tfl_end_col = buf->len + TR2FMT_PERF_FL_WIDTH;\n+\n+\t\tif (file && *file) {\n+\t\t\tstruct strbuf buf_fl = STRBUF_INIT;\n+\n+\t\t\tstrbuf_addf(&buf_fl, \"%s:%d\", file, line);\n+\n+\t\t\tif (buf_fl.len <= TR2FMT_PERF_FL_WIDTH)\n+\t\t\t\tstrbuf_addbuf(buf, &buf_fl);\n+\t\t\telse {\n+\t\t\t\tsize_t avail = TR2FMT_PERF_FL_WIDTH - 3;\n+\t\t\t\tstrbuf_addstr(buf, \"...\");\n+\t\t\t\tstrbuf_add(buf,\n+\t\t\t\t\t &buf_fl.buf[buf_fl.len - avail],\n+\t\t\t\t\t avail);\n+\t\t\t}\n+\n+\t\t\tstrbuf_release(&buf_fl);\n+\t\t}\n+\n+\t\twhile (buf->len < fl_end_col)\n \t\t\tstrbuf_addch(buf, ' ');\n \n-\t\tstrbuf_addstr(buf, \"| \");\n+\t\tstrbuf_addstr(buf, \" | \");\n \t}\n \n \tstrbuf_addf(buf, \"d%d | \", tr2_sid_depth());\n@@ -102,7 +121,7 @@ static void perf_fmt_prepare(const char *event_name,\n \t\tstrbuf_addf(buf, \"r%d \", repo->trace2_repo_id);\n \twhile (buf->len < len)\n \t\tstrbuf_addch(buf, ' ');\n-\tstrbuf_addstr(buf, \"| \");\n+\tstrbuf_addstr(buf, \" | \");\n \n \tif (p_us_elapsed_absolute)\n \t\tstrbuf_addf(buf, \"%9.6f | \",\n@@ -116,8 +135,8 @@ static void perf_fmt_prepare(const char *event_name,\n \telse\n \t\tstrbuf_addf(buf, \"%9s | \", \" \");\n \n-\tstrbuf_addf(buf, \"%-*s | \", TR2FMT_PERF_CATEGORY_WIDTH,\n-\t\t (category ? category : \"\"));\n+\tstrbuf_addf(buf, \"%-*.*s | \", TR2FMT_PERF_CATEGORY_WIDTH,\n+\t\t TR2FMT_PERF_CATEGORY_WIDTH, (category ? category : \"\"));\n \n \tif (ctx->nr_open_regions > 0) {\n \t\tint len_indent = TR2_INDENT_LENGTH(ctx);\n-- \ngitgitgadget\n\n"},{"id":"380216","messageId":"db3509971281bbaadf2003c3b3fa3058ca5f9ab0.1565362851.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 2/7] trace2: trim whitespace in region messages in perf target format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:53Z","receivedAt":"2019-08-09T15:00:57Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nAvoid unecessary trailing whitespace in \"region_enter\" and \"region_leave\"\nmessages in perf target format.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_perf.c | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex 4a9d99218b..fb845cb627 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -452,8 +452,11 @@ static void fn_region_enter_printf_va_fl(const char *file, int line,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tif (label)\n-\t\tstrbuf_addf(&buf_payload, \"label:%s \", label);\n-\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t\tstrbuf_addf(&buf_payload, \"label:%s\", label);\n+\tif (fmt && *fmt) {\n+\t\tstrbuf_addch(&buf_payload, ' ');\n+\t\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t}\n \n \tperf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,\n \t\t\t NULL, category, &buf_payload);\n@@ -469,8 +472,11 @@ static void fn_region_leave_printf_va_fl(\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tif (label)\n-\t\tstrbuf_addf(&buf_payload, \"label:%s \", label);\n-\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t\tstrbuf_addf(&buf_payload, \"label:%s\", label);\n+\tif (fmt && *fmt) {\n+\t\tstrbuf_addch(&buf_payload, ' ' );\n+\t\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t}\n \n \tperf_io_write_fl(file, line, event_name, repo, &us_elapsed_absolute,\n \t\t\t &us_elapsed_region, category, &buf_payload);\n-- \ngitgitgadget\n\n"},{"id":"380215","messageId":"d5b5a70c1b0012e1fca29cdcbefa9df2155bb34b.1565362851.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 4/7] trace2: trim trailing whitespace in normal format error message","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:55Z","receivedAt":"2019-08-09T15:01:00Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nAvoid creating unnecessary trailing whitespace in normal\ntarget format error messages when the message is omitted.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_normal.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\nindex 47a1882557..213724d5cb 100644\n--- a/trace2/tr2_tgt_normal.c\n+++ b/trace2/tr2_tgt_normal.c\n@@ -142,8 +142,11 @@ static void fn_error_va_fl(const char *file, int line, const char *fmt,\n {\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addstr(&buf_payload, \"error \");\n-\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\tstrbuf_addstr(&buf_payload, \"error\");\n+\tif (fmt && *fmt) {\n+\t\tstrbuf_addch(&buf_payload, ' ');\n+\t\tmaybe_append_string_va(&buf_payload, fmt, ap);\n+\t}\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n-- \ngitgitgadget\n\n"},{"id":"380217","messageId":"d02a0608d7ba7902da3b7622ee2c4619eba6896c.1565362851.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 6/7] trace2: cleanup whitespace in normal format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:56Z","receivedAt":"2019-08-09T15:01:01Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nMake use of new sq_append_quote_argv_pretty() to normalize\nhow we handle leading whitespace in normal format messages.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_normal.c | 23 +++++++++++++----------\n 1 file changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\nindex 213724d5cb..438ed05a40 100644\n--- a/trace2/tr2_tgt_normal.c\n+++ b/trace2/tr2_tgt_normal.c\n@@ -87,7 +87,7 @@ static void fn_start_fl(const char *file, int line,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tstrbuf_addstr(&buf_payload, \"start \");\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n@@ -186,8 +186,8 @@ static void fn_alias_fl(const char *file, int line, const char *alias,\n {\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addf(&buf_payload, \"alias %s ->\", alias);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addf(&buf_payload, \"alias %s -> \", alias);\n+\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n@@ -198,12 +198,12 @@ static void fn_child_start_fl(const char *file, int line,\n {\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addf(&buf_payload, \"child_start[%d] \", cmd->trace2_child_id);\n+\tstrbuf_addf(&buf_payload, \"child_start[%d]\", cmd->trace2_child_id);\n \n \tif (cmd->dir) {\n-\t\tstrbuf_addstr(&buf_payload, \" cd\");\n+\t\tstrbuf_addstr(&buf_payload, \" cd \");\n \t\tsq_quote_buf_pretty(&buf_payload, cmd->dir);\n-\t\tstrbuf_addstr(&buf_payload, \"; \");\n+\t\tstrbuf_addstr(&buf_payload, \";\");\n \t}\n \n \t/*\n@@ -211,9 +211,10 @@ static void fn_child_start_fl(const char *file, int line,\n \t * See trace_add_env() in run-command.c as used by original trace.c\n \t */\n \n+\tstrbuf_addch(&buf_payload, ' ');\n \tif (cmd->git_cmd)\n-\t\tstrbuf_addstr(&buf_payload, \"git\");\n-\tsq_quote_argv_pretty(&buf_payload, cmd->argv);\n+\t\tstrbuf_addstr(&buf_payload, \"git \");\n+\tsq_append_quote_argv_pretty(&buf_payload, cmd->argv);\n \n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n@@ -238,9 +239,11 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tstrbuf_addf(&buf_payload, \"exec[%d] \", exec_id);\n-\tif (exe)\n+\tif (exe) {\n \t\tstrbuf_addstr(&buf_payload, exe);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\t\tstrbuf_addch(&buf_payload, ' ');\n+\t}\n+\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \tnormal_io_write_fl(file, line, &buf_payload);\n \tstrbuf_release(&buf_payload);\n }\n-- \ngitgitgadget\n\n"},{"id":"380218","messageId":"9ab73939855f78563b1554282920aa11ff958f4e.1565362851.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 7/7] trace2: cleanup whitespace in perf format","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:57Z","receivedAt":"2019-08-09T15:01:02Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nMake use of new sq_append_quote_argv_pretty() to normalize\nhow we handle leading whitespace in perf format messages.\n\nSigned-off-by: Jeff Hostetler \n---\n t/t0211-trace2-perf.sh | 4 ++--\n trace2/tr2_tgt_perf.c | 31 ++++++++++++++++++++-----------\n 2 files changed, 22 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t0211-trace2-perf.sh b/t/t0211-trace2-perf.sh\nindex 2c3ad6e8c1..6ee8ee3b67 100755\n--- a/t/t0211-trace2-perf.sh\n+++ b/t/t0211-trace2-perf.sh\n@@ -130,11 +130,11 @@ test_expect_success 'perf stream, child processes' '\n \t\td0|main|version|||||$V\n \t\td0|main|start||_T_ABS_|||_EXE_ trace2 004child test-tool trace2 004child test-tool trace2 001return 0\n \t\td0|main|cmd_name|||||trace2 (trace2)\n-\t\td0|main|child_start||_T_ABS_|||[ch0] class:? argv: test-tool trace2 004child test-tool trace2 001return 0\n+\t\td0|main|child_start||_T_ABS_|||[ch0] class:? argv:[test-tool trace2 004child test-tool trace2 001return 0]\n \t\td1|main|version|||||$V\n \t\td1|main|start||_T_ABS_|||_EXE_ trace2 004child test-tool trace2 001return 0\n \t\td1|main|cmd_name|||||trace2 (trace2/trace2)\n-\t\td1|main|child_start||_T_ABS_|||[ch0] class:? argv: test-tool trace2 001return 0\n+\t\td1|main|child_start||_T_ABS_|||[ch0] class:? argv:[test-tool trace2 001return 0]\n \t\td2|main|version|||||$V\n \t\td2|main|start||_T_ABS_|||_EXE_ trace2 001return 0\n \t\td2|main|cmd_name|||||trace2 (trace2/trace2/trace2)\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex 8e52395de2..fd979db4ad 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -184,7 +184,7 @@ static void fn_start_fl(const char *file, int line,\n \tconst char *event_name = \"start\";\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tsq_append_quote_argv_pretty(&buf_payload, argv);\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n@@ -299,8 +299,9 @@ static void fn_alias_fl(const char *file, int line, const char *alias,\n \tconst char *event_name = \"alias\";\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n-\tstrbuf_addf(&buf_payload, \"alias:%s argv:\", alias);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addf(&buf_payload, \"alias:%s argv:[\", alias);\n+\tsq_append_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL,\n \t\t\t &buf_payload);\n@@ -329,10 +330,14 @@ static void fn_child_start_fl(const char *file, int line,\n \t\tsq_quote_buf_pretty(&buf_payload, cmd->dir);\n \t}\n \n-\tstrbuf_addstr(&buf_payload, \" argv:\");\n-\tif (cmd->git_cmd)\n-\t\tstrbuf_addstr(&buf_payload, \" git\");\n-\tsq_quote_argv_pretty(&buf_payload, cmd->argv);\n+\tstrbuf_addstr(&buf_payload, \" argv:[\");\n+\tif (cmd->git_cmd) {\n+\t\tstrbuf_addstr(&buf_payload, \"git\");\n+\t\tif (cmd->argv[0])\n+\t\t\tstrbuf_addch(&buf_payload, ' ');\n+\t}\n+\tsq_append_quote_argv_pretty(&buf_payload, cmd->argv);\n+\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n@@ -383,10 +388,14 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,\n \tstruct strbuf buf_payload = STRBUF_INIT;\n \n \tstrbuf_addf(&buf_payload, \"id:%d \", exec_id);\n-\tstrbuf_addstr(&buf_payload, \"argv:\");\n-\tif (exe)\n-\t\tstrbuf_addf(&buf_payload, \" %s\", exe);\n-\tsq_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addstr(&buf_payload, \"argv:[\");\n+\tif (exe) {\n+\t\tstrbuf_addstr(&buf_payload, exe);\n+\t\tif (argv[0])\n+\t\t\tstrbuf_addch(&buf_payload, ' ');\n+\t}\n+\tsq_append_quote_argv_pretty(&buf_payload, argv);\n+\tstrbuf_addch(&buf_payload, ']');\n \n \tperf_io_write_fl(file, line, event_name, NULL, &us_elapsed_absolute,\n \t\t\t NULL, NULL, &buf_payload);\n-- \ngitgitgadget\n"},{"id":"380219","messageId":"09dff12a7dfe2219feab85b3fe611aa5c4196375.1565362851.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 3/7] trace2: remove dead code in maybe_add_string_va()","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:54Z","receivedAt":"2019-08-09T15:01:03Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nRemove an unnecessary \"if\" block in maybe_add_string_va().\n\nCommit \"ad006fe419e trace2: NULL is not allowed for va_list\"\nchanged \"if (fmt && *fmt && ap)\" to just \"if (fmt && *fmt)\"\nbecause it isn't safe to treat 'ap' as a pointer. This made\nthe \"if\" block following it unnecessary.\n\nSigned-off-by: Jeff Hostetler \n---\n trace2/tr2_tgt_event.c | 5 -----\n trace2/tr2_tgt_normal.c | 5 -----\n trace2/tr2_tgt_perf.c | 5 -----\n 3 files changed, 15 deletions(-)\n\ndiff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c\nindex c2852d1bd2..9bcac20d1b 100644\n--- a/trace2/tr2_tgt_event.c\n+++ b/trace2/tr2_tgt_event.c\n@@ -205,11 +205,6 @@ static void maybe_add_string_va(struct json_writer *jw, const char *field_name,\n \t\tstrbuf_release(&buf);\n \t\treturn;\n \t}\n-\n-\tif (fmt && *fmt) {\n-\t\tjw_object_string(jw, field_name, fmt);\n-\t\treturn;\n-\t}\n }\n \n static void fn_error_va_fl(const char *file, int line, const char *fmt,\ndiff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\nindex 00b116d797..47a1882557 100644\n--- a/trace2/tr2_tgt_normal.c\n+++ b/trace2/tr2_tgt_normal.c\n@@ -135,11 +135,6 @@ static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n \t\tva_end(copy_ap);\n \t\treturn;\n \t}\n-\n-\tif (fmt && *fmt) {\n-\t\tstrbuf_addstr(buf, fmt);\n-\t\treturn;\n-\t}\n }\n \n static void fn_error_va_fl(const char *file, int line, const char *fmt,\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex fb845cb627..8e52395de2 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -239,11 +239,6 @@ static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n \t\tva_end(copy_ap);\n \t\treturn;\n \t}\n-\n-\tif (fmt && *fmt) {\n-\t\tstrbuf_addstr(buf, fmt);\n-\t\treturn;\n-\t}\n }\n \n static void fn_error_va_fl(const char *file, int line, const char *fmt,\n-- \ngitgitgadget\n\n"},{"id":"380220","messageId":"2002108a30246824f8017da1dea344c1a96f7de2.1565362851.git.gitgitgadget@gmail.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 5/7] quote: add sq_append_quote_argv_pretty()","fromName":"Jeff Hostetler via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-08-09T15:00:55Z","receivedAt":"2019-08-09T15:01:05Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nsq_quote_argv_pretty() builds a \"pretty\" string from the given argv.\nIt inserts whitespace before each value, rather than just between\nthem, so the resulting string always has a leading space. Lets give\ncallers an option to not have the leading space or have to ltrim()\nit later.\n\nCreate sq_append_quote_argv_pretty() to convert an argv into a\npretty, quoted if necessary, string with space delimiters and\nwithout a leading space.\n\nConvert the existing sq_quote_argv_pretty() to use this new routine\nwhile preserving the leading space behavior.\n\nSigned-off-by: Jeff Hostetler \n---\n quote.c | 18 +++++++++++++++++-\n quote.h | 1 +\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/quote.c b/quote.c\nindex 7f2aa6faa4..c8ba6b397a 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -84,12 +84,28 @@ void sq_quote_argv(struct strbuf *dst, const char **argv)\n \t}\n }\n \n+/*\n+ * Legacy function to append each argv value, quoted as necessasry,\n+ * with whitespace before each value. This results in a leading\n+ * space in the result.\n+ */\n void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n+{\n+\tif (argv[0])\n+\t\tstrbuf_addch(dst, ' ');\n+\tsq_append_quote_argv_pretty(dst, argv);\n+}\n+\n+/*\n+ * Append each argv value, quoted as necessary, with whitespace between them.\n+ */\n+void sq_append_quote_argv_pretty(struct strbuf *dst, const char **argv)\n {\n \tint i;\n \n \tfor (i = 0; argv[i]; i++) {\n-\t\tstrbuf_addch(dst, ' ');\n+\t\tif (i > 0)\n+\t\t\tstrbuf_addch(dst, ' ');\n \t\tsq_quote_buf_pretty(dst, argv[i]);\n \t}\n }\ndiff --git a/quote.h b/quote.h\nindex fb08dc085c..ca8ee3144a 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -40,6 +40,7 @@ void sq_quotef(struct strbuf *, const char *fmt, ...);\n */\n void sq_quote_buf_pretty(struct strbuf *, const char *src);\n void sq_quote_argv_pretty(struct strbuf *, const char **argv);\n+void sq_append_quote_argv_pretty(struct strbuf *dst, const char **argv);\n \n /* This unwraps what sq_quote() produces in place, but returns\n * NULL if the input does not look like what sq_quote would have\n-- \ngitgitgadget\n\n"},{"id":"380226","messageId":"5651286b-cd28-68ba-4743-98d546dfa3b0@jeffhostetler.com","threadId":"51565","inReplyTo":"60c7b26c-e598-182b-421e-bdf5c0a8ac9f@web.de","subject":"Re: [PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2019-08-09T17:13:25Z","receivedAt":"2019-08-09T17:13:30Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 8/8/2019 6:49 PM, René Scharfe wrote:\n> Am 08.08.19 um 21:04 schrieb Jeff Hostetler:\n>> On 8/8/2019 2:05 PM, Junio C Hamano wrote:\n>>> Having made the primary purpose of the helper clearer leads me to\n>>> wonder if \"do not add SP before the first element, i.e. argv[0]\", is\n>>> really what we want.  If we always clear the *dst strbuf before\n>>> starting to serialize argv[] into it, then the behaviour would make\n>>> sense, but we do not---we are \"appending\".\n>>>\n>>> As long as we are appending, would we be better off doing something\n>>> sillily magical like this instead, I have to wonder?\n>>>\n>>>     void sq_append_strings_quoted(struct strbuf *buf, const char **av)\n>>>     {\n>>>         int i;\n>>>\n>>>         for (i = 0; av[i]; i++) {\n>>>             if (buf->len)\n>>>                 strbuf_addch(buf, ' ');\n>>>             sq_quote_buf_pretty(buf, argv[0]);\n>>>         }\n>>>     }\n>>>\n>>> That is, \"if we are appending to an existing string, have SP to\n>>> separate the first element from that existing string; treat the\n>>> remaining elements the same way (if the buffer is empty, there is no\n>>> point adding SP at the beginning)\".\n>>\n>> I don't think that would do what we want.  We don't know what the\n>> caller's expectations are.  In my uses in commits 6/7 and 7/7 I\n>> already added the leading chars I wanted in the strbuf before calling\n>> sq_quote_argv_pretty_ltrim() and assumed the output would be a true\n>> append.  For example:\n>>\n>> +    strbuf_addf(&buf_payload, \"alias:%s argv:[\", alias);\n>> +    sq_quote_argv_pretty_ltrim(&buf_payload, argv);\n>> +    strbuf_addch(&buf_payload, ']');\n>>\n>> I like your suggestion of putting my new function in the _append_\n>> category.  I think I'll add the 3rd arg to this and then it will\n>> be completely specified and I can get rid of the _ltrim suffix.\n> \n> Two observations:\n> \n> If callers want to add something before a joined delimited list, they\n> already can with a strbuf_add* call. No need to add that feature to\n> a function that joins lists.\n> \n> And repetitions of repetitions (loops) are boring.\n> \n> Apologies in advance for any coffee stains on your monitor, but\n> here's how I would start, probably followed by attempts to inline the\n> functions that become trivial wrappers:\n\n\nUm, yeah, I must say that I didn't expect the conversation to turn to\nmap-style functions and a change in design styles. I think it would be\nbetter to have that conversation in a different patch series and not mix\nit with my trace2 janitoring.\n\nI'm going to push a V3 that does just the minimum to have a sq_ function\nthat joins the args with a space delimiter (and without the leading\nspace) and re-write the existing function to call it after adding the\nlegacy leading space. This will let existing callers continue to work\nas is. And they can be converted if/when anyone wants to dig into them.\n\n\n> \n> ---\n> quote.c | 18 ++++--------------\n> strbuf.c | 20 +++++++++++++-------\n> strbuf.h | 8 ++++++++\n> 3 files changed, 25 insertions(+), 21 deletions(-)\n> \n> diff --git a/quote.c b/quote.c\n> index 7f2aa6faa4..f422188852 100644\n> --- a/quote.c\n> +++ b/quote.c\n> @@ -74,24 +74,14 @@ void sq_quotef(struct strbuf *dst, const char *fmt, ...)\n> \n> void sq_quote_argv(struct strbuf *dst, const char **argv)\n> {\n> -\tint i;\n> -\n> -\t/* Copy into destination buffer. */\n> -\tstrbuf_grow(dst, 255);\n> -\tfor (i = 0; argv[i]; ++i) {\n> -\t\tstrbuf_addch(dst, ' ');\n> -\t\tsq_quote_buf(dst, argv[i]);\n> -\t}\n> +\tstrbuf_addch(dst, ' ');\n> +\tstrbuf_map_join_argv(dst, argv, sq_quote_buf, \" \");\n> }\n> \n> void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n> {\n> -\tint i;\n> -\n> -\tfor (i = 0; argv[i]; i++) {\n> -\t\tstrbuf_addch(dst, ' ');\n> -\t\tsq_quote_buf_pretty(dst, argv[i]);\n> -\t}\n> +\tstrbuf_addch(dst, ' ');\n\nIf I'm reading this correctly, this has slightly different behavior\nthan the original version. Perhaps:\n\n\tif (argv[0])\n\t\tstrbuf_addch(dst, ' ');\n\n> +\tstrbuf_map_join_argv(dst, argv, sq_quote_buf_pretty, \" \");\n> }\n> \n> static char *sq_dequote_step(char *arg, char **next)\n> diff --git a/strbuf.c b/strbuf.c\n> index d30f916858..d337853b53 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -304,17 +304,23 @@ void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2)\n> \tstrbuf_setlen(sb, sb->len + sb2->len);\n> }\n> \n> +void strbuf_map_join_argv(struct strbuf *sb, const char **argv,\n> +\t\t\t void (*fn)(struct strbuf *, const char *),\n> +\t\t\t const char *separator)\n> +{\n> +\twhile (*argv) {\n> +\t\tfn(sb, *argv++);\n> +\t\tif (*argv)\n> +\t\t\tstrbuf_addstr(sb, separator);\n> +\t}\n> +}\n> +\n> const char *strbuf_join_argv(struct strbuf *buf,\n> \t\t\t int argc, const char **argv, char delim)\n> {\n> -\tif (!argc)\n> -\t\treturn buf->buf;\n> +\tchar separator[] = { delim, '\\0' };\n> \n> -\tstrbuf_addstr(buf, *argv);\n> -\twhile (--argc) {\n> -\t\tstrbuf_addch(buf, delim);\n> -\t\tstrbuf_addstr(buf, *(++argv));\n> -\t}\n> +\tstrbuf_map_join_argv(buf, argv, strbuf_addstr, separator);\n> \n> \treturn buf->buf;\n> }\n> diff --git a/strbuf.h b/strbuf.h\n> index f62278a0be..7adeff94a7 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -297,6 +297,14 @@ static inline void strbuf_addstr(struct strbuf *sb, const char *s)\n> */\n> void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2);\n> \n> +/**\n> + * Apply `fn` to `sb` and each element of the NULL-terminated array\n> + * `argv`. Add `separator` between these invocations.\n> + */\n> +void strbuf_map_join_argv(struct strbuf *sb, const char **argv,\n> +\t\t\t void (*fn)(struct strbuf *, const char *),\n> +\t\t\t const char *separator);\n> +\n> /**\n> * Join the arguments into a buffer. `delim` is put between every\n> * two arguments.\n> --\n> 2.22.0\n> \n"},{"id":"380231","messageId":"xmqqtvaqcjhx.fsf@gitster-ct.c.googlers.com","threadId":"51565","inReplyTo":"2002108a30246824f8017da1dea344c1a96f7de2.1565362851.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 5/7] quote: add sq_append_quote_argv_pretty()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-09T17:54:18Z","receivedAt":"2019-08-09T17:54:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jeff Hostetler via GitGitGadget\" writes:\n\n> +/*\n> + * Legacy function to append each argv value, quoted as necessasry,\n> + * with whitespace before each value. This results in a leading\n> + * space in the result.\n> + */\n> void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n> +{\n> +\tif (argv[0])\n> +\t\tstrbuf_addch(dst, ' ');\n> +\tsq_append_quote_argv_pretty(dst, argv);\n\nMicronit (not worth a reroll): if !argv[0], there is no need to call\nappend, either.\n"},{"id":"380232","messageId":"b3cf1b32-9e80-c9c5-ac9c-33f68ef8909d@web.de","threadId":"51565","inReplyTo":"5651286b-cd28-68ba-4743-98d546dfa3b0@jeffhostetler.com","subject":"Re: [PATCH v2 5/7] quote: add sq_quote_argv_pretty_ltrim","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-08-09T18:01:42Z","receivedAt":"2019-08-09T18:01:56Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 09.08.19 um 19:13 schrieb Jeff Hostetler:\n> On 8/8/2019 6:49 PM, René Scharfe wrote:\n>>   void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n>>   {\n>> -    int i;\n>> -\n>> -    for (i = 0; argv[i]; i++) {\n>> -        strbuf_addch(dst, ' ');\n>> -        sq_quote_buf_pretty(dst, argv[i]);\n>> -    }\n>> +    strbuf_addch(dst, ' ');\n>\n> If I'm reading this correctly, this has slightly different behavior\n> than the original version.  Perhaps:\n>\n>     if (argv[0])\n>         strbuf_addch(dst, ' ');\n\nOh, yes, thanks for spotting this.\n\nRené\n"},{"id":"380234","messageId":"xmqqpnlecijw.fsf@gitster-ct.c.googlers.com","threadId":"51565","inReplyTo":"pull.298.v3.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/7] trace2: clean up formatting in perf target format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-09T18:14:43Z","receivedAt":"2019-08-09T18:14:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. Will queue.\n\n"},{"id":"380235","messageId":"2c53b342-f4be-55b0-d14f-16b12826d2a8@jeffhostetler.com","threadId":"51565","inReplyTo":"xmqqtvaqcjhx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 5/7] quote: add sq_append_quote_argv_pretty()","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2019-08-09T18:17:33Z","receivedAt":"2019-08-09T18:17:36Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 8/9/2019 1:54 PM, Junio C Hamano wrote:\n> \"Jeff Hostetler via GitGitGadget\" writes:\n> \n>> +/*\n>> + * Legacy function to append each argv value, quoted as necessasry,\n>> + * with whitespace before each value. This results in a leading\n>> + * space in the result.\n>> + */\n>> void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)\n>> +{\n>> +\tif (argv[0])\n>> +\t\tstrbuf_addch(dst, ' ');\n>> +\tsq_append_quote_argv_pretty(dst, argv);\n> \n> Micronit (not worth a reroll): if !argv[0], there is no need to call\n> append, either.\n> \n\ngood catch. thanks!\n"}]}