{"thread":{"id":"50779","subject":"[PATCH v2 1/1] trace2: NULL is not allowed for va_list","startedAt":"2019-03-19T17:13:56Z","lastAt":"2019-03-19T18:03:35Z","messageCount":2,"participants":["tboegi@web.de","Jeff Hostetler"],"isPatch":true,"patchVersion":2,"patchTotal":1},"messages":[{"id":"371913","messageId":"20190319171347.7677-1-tboegi@web.de","threadId":"50779","inReplyTo":"20190316104616.27085-1-tboegi@web.de","subject":"[PATCH v2 1/1] trace2: NULL is not allowed for va_list","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2019-03-19T17:13:46Z","receivedAt":"2019-03-19T17:13:56Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen <tboegi@web.de>\n\nSome compilers don't allow NULL to be passed for a va_list,\nand e.g. \"gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516\"\nerrors out like this:\n trace2/tr2_tgt_event.c:193:18:\n   error: invalid operands to binary &&\n   (have ‘int’ and ‘va_list {aka __va_list}’)\n    if (fmt && *fmt && ap) {\n                       ^^\nI couldn't find any hints that va_list and pointers can be mixed,\nand no hints that they can't either. Morten Welinder comments:\n\n\"C99, Section 7.15, simply says that va_list \"is an object type suitable for\nholding information needed by the macros va_start, va_end, and\nva_copy\". So clearly not guaranteed to be mixable with pointers...\n\nThe portable solution is to use \"va_list\" everywhere in the callchain.\nAs a consequence, both trace2_region_enter_fl() and trace2_region_leave_fl()\nnow take a variable argument list.\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\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":"371914","messageId":"1899eb63-c986-6c18-d09e-9b1584f471d0@jeffhostetler.com","threadId":"50779","inReplyTo":"20190319171347.7677-1-tboegi@web.de","subject":"Re: [PATCH v2 1/1] trace2: NULL is not allowed for va_list","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2019-03-19T18:03:32Z","receivedAt":"2019-03-19T18:03:35Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 3/19/2019 1:13 PM, tboegi@web.de wrote:\n> From: Torsten Bögershausen <tboegi@web.de>\n> \n> Some compilers don't allow NULL to be passed for a va_list,\n> and e.g. \"gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516\"\n> errors out like this:\n>   trace2/tr2_tgt_event.c:193:18:\n>     error: invalid operands to binary &&\n>     (have ‘int’ and ‘va_list {aka __va_list}’)\n>      if (fmt && *fmt && ap) {\n>                         ^^\n> I couldn't find any hints that va_list and pointers can be mixed,\n> and no hints that they can't either. Morten Welinder comments:\n> \n> \"C99, Section 7.15, simply says that va_list \"is an object type suitable for\n> holding information needed by the macros va_start, va_end, and\n> va_copy\". So clearly not guaranteed to be mixable with pointers...\n> \n> The portable solution is to use \"va_list\" everywhere in the callchain.\n> As a consequence, both trace2_region_enter_fl() and trace2_region_leave_fl()\n> now take a variable argument list.\n> \n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n\nLooks good.  Thanks for tracking this down.\nJeff\n\n\nSigned-off-by: Jeff Hostetler <jeffhost@microsoft.com>\n"}]}