{"thread":{"id":"50756","subject":"[PATCH v1 1/1] trace2: NULL is not allowed for va_list","startedAt":"2019-03-16T10:47:22Z","lastAt":"2019-03-18T17:58:20Z","messageCount":5,"participants":["tboegi@web.de","Junio C Hamano","Jeff Hostetler","Torsten Bögershausen","Morten Welinder"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"371650","messageId":"20190316104715.27138-1-tboegi@web.de","threadId":"50756","inReplyTo":null,"subject":"[PATCH v1 1/1] trace2: NULL is not allowed for va_list","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2019-03-16T10:47:15Z","receivedAt":"2019-03-16T10:47:22Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nSome compilers don't allow NULL to be passed for a va_list.\nUse va_list instead.\n\nSigned-off-by: Torsten Bögershausen \n---\n trace2.c | 15 +++++++++++----\n trace2.h | 4 ++--\n trace2/tr2_tgt_event.c | 2 +-\n trace2/tr2_tgt_normal.c | 2 +-\n trace2/tr2_tgt_perf.c | 2 +-\n 5 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/trace2.c b/trace2.c\nindex ccccd4ef09..8bbad56887 100644\n--- a/trace2.c\n+++ b/trace2.c\n@@ -548,10 +548,14 @@ void trace2_region_enter_printf_va_fl(const char *file, int line,\n }\n\n void trace2_region_enter_fl(const char *file, int line, const char *category,\n-\t\t\t const char *label, const struct repository *repo)\n+\t\t\t const char *label, const struct repository *repo, ...)\n {\n+\tva_list ap;\n+\tva_start(ap, repo);\n \ttrace2_region_enter_printf_va_fl(file, line, category, label, repo,\n-\t\t\t\t\t NULL, NULL);\n+\t\t\t\t\t NULL, ap);\n+\tva_end(ap);\n+\n }\n\n void trace2_region_enter_printf_fl(const char *file, int line,\n@@ -621,10 +625,13 @@ void trace2_region_leave_printf_va_fl(const char *file, int line,\n }\n\n void trace2_region_leave_fl(const char *file, int line, const char *category,\n-\t\t\t const char *label, const struct repository *repo)\n+\t\t\t const char *label, const struct repository *repo, ...)\n {\n+\tva_list ap;\n+\tva_start(ap, repo);\n \ttrace2_region_leave_printf_va_fl(file, line, category, label, repo,\n-\t\t\t\t\t NULL, NULL);\n+\t\t\t\t\t NULL, ap);\n+\tva_end(ap);\n }\n\n void trace2_region_leave_printf_fl(const char *file, int line,\ndiff --git a/trace2.h b/trace2.h\nindex ae5020d0e6..b330a54a89 100644\n--- a/trace2.h\n+++ b/trace2.h\n@@ -238,7 +238,7 @@ void trace2_def_repo_fl(const char *file, int line, struct repository *repo);\n * on this thread.\n */\n void trace2_region_enter_fl(const char *file, int line, const char *category,\n-\t\t\t const char *label, const struct repository *repo);\n+\t\t\t const char *label, const struct repository *repo, ...);\n\n #define trace2_region_enter(category, label, repo) \\\n \ttrace2_region_enter_fl(__FILE__, __LINE__, (category), (label), (repo))\n@@ -278,7 +278,7 @@ void trace2_region_enter_printf(const char *category, const char *label,\n * in this nesting level.\n */\n void trace2_region_leave_fl(const char *file, int line, const char *category,\n-\t\t\t const char *label, const struct repository *repo);\n+\t\t\t const char *label, const struct repository *repo, ...);\n\n #define trace2_region_leave(category, label, repo) \\\n \ttrace2_region_leave_fl(__FILE__, __LINE__, (category), (label), (repo))\ndiff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c\nindex 107cb5317d..1cf4f62441 100644\n--- a/trace2/tr2_tgt_event.c\n+++ b/trace2/tr2_tgt_event.c\n@@ -190,7 +190,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)\n static void maybe_add_string_va(struct json_writer *jw, const char *field_name,\n \t\t\t\tconst char *fmt, va_list ap)\n {\n-\tif (fmt && *fmt && ap) {\n+\tif (fmt && *fmt) {\n \t\tva_list copy_ap;\n \t\tstruct strbuf buf = STRBUF_INIT;\n\ndiff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\nindex 547183d5b6..1a07d70abd 100644\n--- a/trace2/tr2_tgt_normal.c\n+++ b/trace2/tr2_tgt_normal.c\n@@ -126,7 +126,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)\n static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n \t\t\t\t va_list ap)\n {\n-\tif (fmt && *fmt && ap) {\n+\tif (fmt && *fmt) {\n \t\tva_list copy_ap;\n\n \t\tva_copy(copy_ap, ap);\ndiff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\nindex f0746fcf86..2a866d701b 100644\n--- a/trace2/tr2_tgt_perf.c\n+++ b/trace2/tr2_tgt_perf.c\n@@ -211,7 +211,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)\n static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n \t\t\t\t va_list ap)\n {\n-\tif (fmt && *fmt && ap) {\n+\tif (fmt && *fmt) {\n \t\tva_list copy_ap;\n\n \t\tva_copy(copy_ap, ap);\n--\n2.21.0.135.g6e0cc67761\n\n"},{"id":"371780","messageId":"xmqqlg1c3dtq.fsf@gitster-ct.c.googlers.com","threadId":"50756","inReplyTo":"20190316104715.27138-1-tboegi@web.de","subject":"Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-18T06:18:41Z","receivedAt":"2019-03-18T06:18:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> From: Torsten Bögershausen \n>\n> Some compilers don't allow NULL to be passed for a va_list.\n> Use va_list instead.\n\nWow (I seem to be keep saying this today).\n\n>\n> Signed-off-by: Torsten Bögershausen \n> ---\n> trace2.c | 15 +++++++++++----\n> trace2.h | 4 ++--\n> trace2/tr2_tgt_event.c | 2 +-\n> trace2/tr2_tgt_normal.c | 2 +-\n> trace2/tr2_tgt_perf.c | 2 +-\n> 5 files changed, 16 insertions(+), 9 deletions(-)\n>\n> diff --git a/trace2.c b/trace2.c\n> index ccccd4ef09..8bbad56887 100644\n> --- a/trace2.c\n> +++ b/trace2.c\n> @@ -548,10 +548,14 @@ void trace2_region_enter_printf_va_fl(const char *file, int line,\n> }\n>\n> void trace2_region_enter_fl(const char *file, int line, const char *category,\n> -\t\t\t const char *label, const struct repository *repo)\n> +\t\t\t const char *label, const struct repository *repo, ...)\n> {\n> +\tva_list ap;\n> +\tva_start(ap, repo);\n> \ttrace2_region_enter_printf_va_fl(file, line, category, label, repo,\n> -\t\t\t\t\t NULL, NULL);\n> +\t\t\t\t\t NULL, ap);\n> +\tva_end(ap);\n> +\n> }\n>\n> void trace2_region_enter_printf_fl(const char *file, int line,\n> @@ -621,10 +625,13 @@ void trace2_region_leave_printf_va_fl(const char *file, int line,\n> }\n>\n> void trace2_region_leave_fl(const char *file, int line, const char *category,\n> -\t\t\t const char *label, const struct repository *repo)\n> +\t\t\t const char *label, const struct repository *repo, ...)\n> {\n> +\tva_list ap;\n> +\tva_start(ap, repo);\n> \ttrace2_region_leave_printf_va_fl(file, line, category, label, repo,\n> -\t\t\t\t\t NULL, NULL);\n> +\t\t\t\t\t NULL, ap);\n> +\tva_end(ap);\n> }\n>\n> void trace2_region_leave_printf_fl(const char *file, int line,\n> diff --git a/trace2.h b/trace2.h\n> index ae5020d0e6..b330a54a89 100644\n> --- a/trace2.h\n> +++ b/trace2.h\n> @@ -238,7 +238,7 @@ void trace2_def_repo_fl(const char *file, int line, struct repository *repo);\n> * on this thread.\n> */\n> void trace2_region_enter_fl(const char *file, int line, const char *category,\n> -\t\t\t const char *label, const struct repository *repo);\n> +\t\t\t const char *label, const struct repository *repo, ...);\n>\n> #define trace2_region_enter(category, label, repo) \\\n> \ttrace2_region_enter_fl(__FILE__, __LINE__, (category), (label), (repo))\n> @@ -278,7 +278,7 @@ void trace2_region_enter_printf(const char *category, const char *label,\n> * in this nesting level.\n> */\n> void trace2_region_leave_fl(const char *file, int line, const char *category,\n> -\t\t\t const char *label, const struct repository *repo);\n> +\t\t\t const char *label, const struct repository *repo, ...);\n>\n> #define trace2_region_leave(category, label, repo) \\\n> \ttrace2_region_leave_fl(__FILE__, __LINE__, (category), (label), (repo))\n> diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c\n> index 107cb5317d..1cf4f62441 100644\n> --- a/trace2/tr2_tgt_event.c\n> +++ b/trace2/tr2_tgt_event.c\n> @@ -190,7 +190,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)\n> static void maybe_add_string_va(struct json_writer *jw, const char *field_name,\n> \t\t\t\tconst char *fmt, va_list ap)\n> {\n> -\tif (fmt && *fmt && ap) {\n> +\tif (fmt && *fmt) {\n> \t\tva_list copy_ap;\n> \t\tstruct strbuf buf = STRBUF_INIT;\n>\n> diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c\n> index 547183d5b6..1a07d70abd 100644\n> --- a/trace2/tr2_tgt_normal.c\n> +++ b/trace2/tr2_tgt_normal.c\n> @@ -126,7 +126,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)\n> static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n> \t\t\t\t va_list ap)\n> {\n> -\tif (fmt && *fmt && ap) {\n> +\tif (fmt && *fmt) {\n> \t\tva_list copy_ap;\n>\n> \t\tva_copy(copy_ap, ap);\n> diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c\n> index f0746fcf86..2a866d701b 100644\n> --- a/trace2/tr2_tgt_perf.c\n> +++ b/trace2/tr2_tgt_perf.c\n> @@ -211,7 +211,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)\n> static void maybe_append_string_va(struct strbuf *buf, const char *fmt,\n> \t\t\t\t va_list ap)\n> {\n> -\tif (fmt && *fmt && ap) {\n> +\tif (fmt && *fmt) {\n> \t\tva_list copy_ap;\n>\n> \t\tva_copy(copy_ap, ap);\n> --\n> 2.21.0.135.g6e0cc67761\n"},{"id":"371809","messageId":"30c8b265-d6bf-3265-b2ae-029aa60d63e5@jeffhostetler.com","threadId":"50756","inReplyTo":"20190316104715.27138-1-tboegi@web.de","subject":"Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2019-03-18T12:35:26Z","receivedAt":"2019-03-18T12:35:37Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 3/16/2019 6:47 AM, tboegi@web.de wrote:\n> From: Torsten Bögershausen \n> \n> Some compilers don't allow NULL to be passed for a va_list.\n> Use va_list instead.\n> \n> Signed-off-by: Torsten Bögershausen \n\nThanks for the fixup.\n\nFor future reference, can you elaborate on which compiler\nand/or platform has the problem ?\n\nJeff\n"},{"id":"371819","messageId":"20190318155459.nrog5r5y3ci3bz3x@tb-raspi4","threadId":"50756","inReplyTo":"30c8b265-d6bf-3265-b2ae-029aa60d63e5@jeffhostetler.com","subject":"Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2019-03-18T15:54:59Z","receivedAt":"2019-03-18T15:55:08Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Mon, Mar 18, 2019 at 08:35:26AM -0400, Jeff Hostetler wrote:\n>\n>\n> On 3/16/2019 6:47 AM, tboegi@web.de wrote:\n> > From: Torsten Bögershausen \n> >\n> > Some compilers don't allow NULL to be passed for a va_list.\n> > Use va_list instead.\n> >\n> > Signed-off-by: Torsten Bögershausen \n>\n> Thanks for the fixup.\n>\n> For future reference, can you elaborate on which compiler\n> and/or platform has the problem ?\n>\n> Jeff\n\nIt is on a Raspberry PI:\ngcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516\n\ntrace2/tr2_tgt_event.c:193:18: error: invalid operands to binary && (have ‘int’ and ‘va_list {aka __va_list}’)\n if (fmt && *fmt && ap) {\n ~~~~~~~~~~~ ^~\n\n(And I couldn't find any hints that va_list and pointers can be mixed,\n and no hints that they can't either)\n\n"},{"id":"371831","messageId":"CANv4PNm_9W9cqUdcx7vxOnafT_BR1grzOwsepLk+m3KRh0btoA@mail.gmail.com","threadId":"50756","inReplyTo":"20190318155459.nrog5r5y3ci3bz3x@tb-raspi4","subject":"Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2019-03-18T17:58:07Z","receivedAt":"2019-03-18T17:58:20Z","isPatch":true,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"> And I couldn't find any hints that va_list and pointers can be mixed,\n> and no hints that they can't either\n\nC99, Section 7.15, simply says that it \"is an object type suitable for\nholding information needed by the macros va_start, va_end, and\nva_copy\".\n\nSo clearly not guaranteed to be mixable with pointers. And not\nprohibited from being mixable either, for that matter.\nAnd, nit-pickingly, NULL might be a valid value representing some\nsequence of arguments if the two are mixable.\n\nM.\n"}]}