# [PATCH v1 1/1] trace2: NULL is not allowed for va_list 5 messages from 2019-03-16 to 2019-03-18. Participants: tboegi@web.de, Junio C Hamano, Jeff Hostetler, Torsten Bögershausen, Morten Welinder. Thread: https://gitlist.dev/t/50756 ## tboegi@web.de, 2019-03-16 10:47 Subject: [PATCH v1 1/1] trace2: NULL is not allowed for va_list Message-ID: <20190316104715.27138-1-tboegi@web.de> URL: https://gitlist.dev/e/20190316104715.27138-1-tboegi%40web.de ``` From: Torsten Bögershausen Some compilers don't allow NULL to be passed for a va_list. Use va_list instead. Signed-off-by: Torsten Bögershausen --- trace2.c | 15 +++++++++++---- trace2.h | 4 ++-- trace2/tr2_tgt_event.c | 2 +- trace2/tr2_tgt_normal.c | 2 +- trace2/tr2_tgt_perf.c | 2 +- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/trace2.c b/trace2.c index ccccd4ef09..8bbad56887 100644 --- a/trace2.c +++ b/trace2.c @@ -548,10 +548,14 @@ void trace2_region_enter_printf_va_fl(const char *file, int line, } void trace2_region_enter_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo) + const char *label, const struct repository *repo, ...) { + va_list ap; + va_start(ap, repo); trace2_region_enter_printf_va_fl(file, line, category, label, repo, - NULL, NULL); + NULL, ap); + va_end(ap); + } void trace2_region_enter_printf_fl(const char *file, int line, @@ -621,10 +625,13 @@ void trace2_region_leave_printf_va_fl(const char *file, int line, } void trace2_region_leave_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo) + const char *label, const struct repository *repo, ...) { + va_list ap; + va_start(ap, repo); trace2_region_leave_printf_va_fl(file, line, category, label, repo, - NULL, NULL); + NULL, ap); + va_end(ap); } void trace2_region_leave_printf_fl(const char *file, int line, diff --git a/trace2.h b/trace2.h index ae5020d0e6..b330a54a89 100644 --- a/trace2.h +++ b/trace2.h @@ -238,7 +238,7 @@ void trace2_def_repo_fl(const char *file, int line, struct repository *repo); * on this thread. */ void trace2_region_enter_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo); + const char *label, const struct repository *repo, ...); #define trace2_region_enter(category, label, repo) \ trace2_region_enter_fl(__FILE__, __LINE__, (category), (label), (repo)) @@ -278,7 +278,7 @@ void trace2_region_enter_printf(const char *category, const char *label, * in this nesting level. */ void trace2_region_leave_fl(const char *file, int line, const char *category, - const char *label, const struct repository *repo); + const char *label, const struct repository *repo, ...); #define trace2_region_leave(category, label, repo) \ trace2_region_leave_fl(__FILE__, __LINE__, (category), (label), (repo)) diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c index 107cb5317d..1cf4f62441 100644 --- a/trace2/tr2_tgt_event.c +++ b/trace2/tr2_tgt_event.c @@ -190,7 +190,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) static void maybe_add_string_va(struct json_writer *jw, const char *field_name, const char *fmt, va_list ap) { - if (fmt && *fmt && ap) { + if (fmt && *fmt) { va_list copy_ap; struct strbuf buf = STRBUF_INIT; diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c index 547183d5b6..1a07d70abd 100644 --- a/trace2/tr2_tgt_normal.c +++ b/trace2/tr2_tgt_normal.c @@ -126,7 +126,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) static void maybe_append_string_va(struct strbuf *buf, const char *fmt, va_list ap) { - if (fmt && *fmt && ap) { + if (fmt && *fmt) { va_list copy_ap; va_copy(copy_ap, ap); diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c index f0746fcf86..2a866d701b 100644 --- a/trace2/tr2_tgt_perf.c +++ b/trace2/tr2_tgt_perf.c @@ -211,7 +211,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) static void maybe_append_string_va(struct strbuf *buf, const char *fmt, va_list ap) { - if (fmt && *fmt && ap) { + if (fmt && *fmt) { va_list copy_ap; va_copy(copy_ap, ap); -- 2.21.0.135.g6e0cc67761 ``` ## Junio C Hamano, 2019-03-18 06:18 Subject: Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list Message-ID: URL: https://gitlist.dev/e/xmqqlg1c3dtq.fsf%40gitster-ct.c.googlers.com In-Reply-To: <20190316104715.27138-1-tboegi@web.de> ``` tboegi@web.de writes: > From: Torsten Bögershausen > > Some compilers don't allow NULL to be passed for a va_list. > Use va_list instead. Wow (I seem to be keep saying this today). > > Signed-off-by: Torsten Bögershausen > --- > trace2.c | 15 +++++++++++---- > trace2.h | 4 ++-- > trace2/tr2_tgt_event.c | 2 +- > trace2/tr2_tgt_normal.c | 2 +- > trace2/tr2_tgt_perf.c | 2 +- > 5 files changed, 16 insertions(+), 9 deletions(-) > > diff --git a/trace2.c b/trace2.c > index ccccd4ef09..8bbad56887 100644 > --- a/trace2.c > +++ b/trace2.c > @@ -548,10 +548,14 @@ void trace2_region_enter_printf_va_fl(const char *file, int line, > } > > void trace2_region_enter_fl(const char *file, int line, const char *category, > - const char *label, const struct repository *repo) > + const char *label, const struct repository *repo, ...) > { > + va_list ap; > + va_start(ap, repo); > trace2_region_enter_printf_va_fl(file, line, category, label, repo, > - NULL, NULL); > + NULL, ap); > + va_end(ap); > + > } > > void trace2_region_enter_printf_fl(const char *file, int line, > @@ -621,10 +625,13 @@ void trace2_region_leave_printf_va_fl(const char *file, int line, > } > > void trace2_region_leave_fl(const char *file, int line, const char *category, > - const char *label, const struct repository *repo) > + const char *label, const struct repository *repo, ...) > { > + va_list ap; > + va_start(ap, repo); > trace2_region_leave_printf_va_fl(file, line, category, label, repo, > - NULL, NULL); > + NULL, ap); > + va_end(ap); > } > > void trace2_region_leave_printf_fl(const char *file, int line, > diff --git a/trace2.h b/trace2.h > index ae5020d0e6..b330a54a89 100644 > --- a/trace2.h > +++ b/trace2.h > @@ -238,7 +238,7 @@ void trace2_def_repo_fl(const char *file, int line, struct repository *repo); > * on this thread. > */ > void trace2_region_enter_fl(const char *file, int line, const char *category, > - const char *label, const struct repository *repo); > + const char *label, const struct repository *repo, ...); > > #define trace2_region_enter(category, label, repo) \ > trace2_region_enter_fl(__FILE__, __LINE__, (category), (label), (repo)) > @@ -278,7 +278,7 @@ void trace2_region_enter_printf(const char *category, const char *label, > * in this nesting level. > */ > void trace2_region_leave_fl(const char *file, int line, const char *category, > - const char *label, const struct repository *repo); > + const char *label, const struct repository *repo, ...); > > #define trace2_region_leave(category, label, repo) \ > trace2_region_leave_fl(__FILE__, __LINE__, (category), (label), (repo)) > diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c > index 107cb5317d..1cf4f62441 100644 > --- a/trace2/tr2_tgt_event.c > +++ b/trace2/tr2_tgt_event.c > @@ -190,7 +190,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) > static void maybe_add_string_va(struct json_writer *jw, const char *field_name, > const char *fmt, va_list ap) > { > - if (fmt && *fmt && ap) { > + if (fmt && *fmt) { > va_list copy_ap; > struct strbuf buf = STRBUF_INIT; > > diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c > index 547183d5b6..1a07d70abd 100644 > --- a/trace2/tr2_tgt_normal.c > +++ b/trace2/tr2_tgt_normal.c > @@ -126,7 +126,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) > static void maybe_append_string_va(struct strbuf *buf, const char *fmt, > va_list ap) > { > - if (fmt && *fmt && ap) { > + if (fmt && *fmt) { > va_list copy_ap; > > va_copy(copy_ap, ap); > diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c > index f0746fcf86..2a866d701b 100644 > --- a/trace2/tr2_tgt_perf.c > +++ b/trace2/tr2_tgt_perf.c > @@ -211,7 +211,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code) > static void maybe_append_string_va(struct strbuf *buf, const char *fmt, > va_list ap) > { > - if (fmt && *fmt && ap) { > + if (fmt && *fmt) { > va_list copy_ap; > > va_copy(copy_ap, ap); > -- > 2.21.0.135.g6e0cc67761 ``` ## Jeff Hostetler, 2019-03-18 12:35 Subject: Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list Message-ID: <30c8b265-d6bf-3265-b2ae-029aa60d63e5@jeffhostetler.com> URL: https://gitlist.dev/e/30c8b265-d6bf-3265-b2ae-029aa60d63e5%40jeffhostetler.com In-Reply-To: <20190316104715.27138-1-tboegi@web.de> ``` On 3/16/2019 6:47 AM, tboegi@web.de wrote: > From: Torsten Bögershausen > > Some compilers don't allow NULL to be passed for a va_list. > Use va_list instead. > > Signed-off-by: Torsten Bögershausen Thanks for the fixup. For future reference, can you elaborate on which compiler and/or platform has the problem ? Jeff ``` ## Torsten Bögershausen, 2019-03-18 15:54 Subject: Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list Message-ID: <20190318155459.nrog5r5y3ci3bz3x@tb-raspi4> URL: https://gitlist.dev/e/20190318155459.nrog5r5y3ci3bz3x%40tb-raspi4 In-Reply-To: <30c8b265-d6bf-3265-b2ae-029aa60d63e5@jeffhostetler.com> ``` On Mon, Mar 18, 2019 at 08:35:26AM -0400, Jeff Hostetler wrote: > > > On 3/16/2019 6:47 AM, tboegi@web.de wrote: > > From: Torsten Bögershausen > > > > Some compilers don't allow NULL to be passed for a va_list. > > Use va_list instead. > > > > Signed-off-by: Torsten Bögershausen > > Thanks for the fixup. > > For future reference, can you elaborate on which compiler > and/or platform has the problem ? > > Jeff It is on a Raspberry PI: gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516 trace2/tr2_tgt_event.c:193:18: error: invalid operands to binary && (have ‘int’ and ‘va_list {aka __va_list}’) if (fmt && *fmt && ap) { ~~~~~~~~~~~ ^~ (And I couldn't find any hints that va_list and pointers can be mixed, and no hints that they can't either) ``` ## Morten Welinder, 2019-03-18 17:58 Subject: Re: [PATCH v1 1/1] trace2: NULL is not allowed for va_list Message-ID: URL: https://gitlist.dev/e/CANv4PNm_9W9cqUdcx7vxOnafT_BR1grzOwsepLk%2Bm3KRh0btoA%40mail.gmail.com In-Reply-To: <20190318155459.nrog5r5y3ci3bz3x@tb-raspi4> ``` > And I couldn't find any hints that va_list and pointers can be mixed, > and no hints that they can't either C99, Section 7.15, simply says that it "is an object type suitable for holding information needed by the macros va_start, va_end, and va_copy". So clearly not guaranteed to be mixable with pointers. And not prohibited from being mixable either, for that matter. And, nit-pickingly, NULL might be a valid value representing some sequence of arguments if the two are mixable. M. ```