trace2: NULL is not allowed for va_list

tboegi · gitster · commit ad006fe419ef · 2019-03-20T09:48:53.000+09:00
Some compilers don't allow NULL to be passed for a va_list,
and e.g. "gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516"
errors out like this:
 trace2/tr2_tgt_event.c:193:18:
   error: invalid operands to binary &amp;&amp;
   (have ‘int’ and ‘va_list {aka __va_list}’)
    if (fmt &amp;&amp; *fmt &amp;&amp; ap) {
                       ^^
I couldn't find any hints that va_list and pointers can be mixed,
and no hints that they can't either. Morten Welinder comments:

"C99, Section 7.15, simply says that va_list "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...

The portable solution is to use "va_list" everywhere in the callchain.
As a consequence, both trace2_region_enter_fl() and trace2_region_leave_fl()
now take a variable argument list.

Signed-off-by: Torsten Bögershausen &lt;tboegi@web.de&gt;
Acked-by: Jeff Hostetler &lt;jeffhost@microsoft.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -548,10 +548,14 @@ void trace2_region_enter_printf_va_fl(const char *file, int line,`
`548`	`548`	`}`
`549`	`549`
`550`	`550`	`void trace2_region_enter_fl(const char file, int line, const char category,`
`551`		`- const char label, const struct repository repo)`
	`551`	`+ const char label, const struct repository repo, ...)`
`552`	`552`	`{`
	`553`	`+ va_list ap;`
	`554`	`+ va_start(ap, repo);`
`553`	`555`	`trace2_region_enter_printf_va_fl(file, line, category, label, repo,`
`554`		`- NULL, NULL);`
	`556`	`+ NULL, ap);`
	`557`	`+ va_end(ap);`
	`558`	`+`
`555`	`559`	`}`
`556`	`560`
`557`	`561`	`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,`
`621`	`625`	`}`
`622`	`626`
`623`	`627`	`void trace2_region_leave_fl(const char file, int line, const char category,`
`624`		`- const char label, const struct repository repo)`
	`628`	`+ const char label, const struct repository repo, ...)`
`625`	`629`	`{`
	`630`	`+ va_list ap;`
	`631`	`+ va_start(ap, repo);`
`626`	`632`	`trace2_region_leave_printf_va_fl(file, line, category, label, repo,`
`627`		`- NULL, NULL);`
	`633`	`+ NULL, ap);`
	`634`	`+ va_end(ap);`
`628`	`635`	`}`
`629`	`636`
`630`	`637`	`void trace2_region_leave_printf_fl(const char *file, int line,`
Original file line number	Diff line number	Diff line change
`@@ -190,7 +190,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)`
`190`	`190`	`static void maybe_add_string_va(struct json_writer jw, const char field_name,`
`191`	`191`	`const char *fmt, va_list ap)`
`192`	`192`	`{`
`193`		`- if (fmt && *fmt && ap) {`
	`193`	`+ if (fmt && *fmt) {`
`194`	`194`	`va_list copy_ap;`
`195`	`195`	`struct strbuf buf = STRBUF_INIT;`
`196`	`196`
Original file line number	Diff line number	Diff line change
`@@ -126,7 +126,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)`
`126`	`126`	`static void maybe_append_string_va(struct strbuf buf, const char fmt,`
`127`	`127`	`va_list ap)`
`128`	`128`	`{`
`129`		`- if (fmt && *fmt && ap) {`
	`129`	`+ if (fmt && *fmt) {`
`130`	`130`	`va_list copy_ap;`
`131`	`131`
`132`	`132`	`va_copy(copy_ap, ap);`
Original file line number	Diff line number	Diff line change
`@@ -211,7 +211,7 @@ static void fn_atexit(uint64_t us_elapsed_absolute, int code)`
`211`	`211`	`static void maybe_append_string_va(struct strbuf buf, const char fmt,`
`212`	`212`	`va_list ap)`
`213`	`213`	`{`
`214`		`- if (fmt && *fmt && ap) {`
	`214`	`+ if (fmt && *fmt) {`
`215`	`215`	`va_list copy_ap;`
`216`	`216`
`217`	`217`	`va_copy(copy_ap, ap);`