@matheusbsbs / matheus-bagliano / commits / 56a29d2c97

C99: remove hardcoded-out !HAVE_VARIADIC_MACROS code

Remove the "else" branches of the HAVE_VARIADIC_MACROS macro, which have been unconditionally omitted since 765dc168882 (git-compat-util: always enable variadic macros, 2021-01-28). Since were always omitted, anyone trying to use a compiler without variadic macro support to compile a git since version git v2.31.0 or later would have had a compilation error. 10 months across a few releases since then should have been enough time for anyone who cared to run into that and report the issue. In addition to that, for anyone unsetting HAVE_VARIADIC_MACROS we've been emitting extremely verbose warnings since at least ee4512ed481 (trace2: create new combined trace facility, 2019-02-22). That's because there is no such thing as a "region_enter_printf" or "region_leave_printf" format, so at least under GCC and Clang everything that includes trace.h (almost every file) emits a couple of warnings about that. There's a large benefit to being able to have a hard dependency rely on variadic macros, the code surrounding usage.c is hard to maintain if we need to write two implementations of everything, and by relying on "__FILE__" and "__LINE__" along with "__VA_ARGS__" we can in the future make error(), die() etc. log where they were called from. We've also recently merged d67fc4bf0ba (Merge branch 'bc/require-c99', 2021-12-10) which further cements our hard dependency on C99. So let's delete the fallback code, and update our CodingGuidelines to note that we depend on this. The added bullet-point starts with lower-case for consistency with other bullet-points in that section. The diff in "trace.h" is relatively hard to read, since we need to retain the existing API docs, which were comments on the code used if HAVE_VARIADIC_MACROS was not defined. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Feb 21, 2022 at 17:05 UTC 56a29d2c970ca9f95fcfb5859a417a68ff7d5b47

8 files changed +64 -243

Documentation/CodingGuidelines

@@ -217,6 +217,9 @@ For C programs:

217 . since mid 2017 with 512f41cf, we have been using designated

218 initializers for array (e.g. "int array[10] = { [5] = 2 }").

219

220 + . since early 2021 with 765dc168882, we have been using variadic

221 + macros, mostly for printf-like trace and debug macros.

222 +

223 These used to be forbidden, but we have not heard any breakage

224 report, and they are assumed to be safe.

225

banned.h

-5

@@ -21,13 +21,8 @@

22 #undef sprintf

23 #undef vsprintf

24 -#ifdef HAVE_VARIADIC_MACROS

24 #define sprintf(...) BANNED(sprintf)

25 #define vsprintf(...) BANNED(vsprintf)

27 -#else

28 -#define sprintf(buf,fmt,arg) BANNED(sprintf)

29 -#define vsprintf(buf,fmt,arg) BANNED(vsprintf)

30 -#endif

27 #undef gmtime

28 #define gmtime(t) BANNED(gmtime)

git-compat-util.h

-12

@@ -1256,24 +1256,12 @@ static inline int regexec_buf(const regex_t *preg, const char *buf, size_t size,

1256 #endif

1257 #endif

1258

1259 -/*

1260 - * This is always defined as a first step towards making the use of variadic

1261 - * macros unconditional. If it causes compilation problems on your platform,

1262 - * please report it to the Git mailing list at git@vger.kernel.org.

1263 - */

1264 -#define HAVE_VARIADIC_MACROS 1

1265 -

1259 /* usage.c: only to be used for testing BUG() implementation (see test-tool) */

1260 extern int BUG_exit_code;

1261

1269 -#ifdef HAVE_VARIADIC_MACROS

1262 __attribute__((format (printf, 3, 4))) NORETURN

1263 void BUG_fl(const char *file, int line, const char *fmt, ...);

1264 #define BUG(...) BUG_fl(__FILE__, __LINE__, __VA_ARGS__)

1273 -#else

1274 -__attribute__((format (printf, 1, 2))) NORETURN

1275 -void BUG(const char *fmt, ...);

1276 -#endif

1265

1266 /*

1267 * Preserves errno, prints a message, but gives no warning for ENOENT.

trace.c

+2 -78

@@ -108,16 +108,11 @@ static int prepare_trace_line(const char *file, int line,

108 gettimeofday(&tv, NULL);

109 secs = tv.tv_sec;

110 localtime_r(&secs, &tm);

111 - strbuf_addf(buf, "%02d:%02d:%02d.%06ld ", tm.tm_hour, tm.tm_min,

112 - tm.tm_sec, (long) tv.tv_usec);

113 -

114 -#ifdef HAVE_VARIADIC_MACROS

115 - /* print file:line */

116 - strbuf_addf(buf, "%s:%d ", file, line);

111 + strbuf_addf(buf, "%02d:%02d:%02d.%06ld %s:%d", tm.tm_hour, tm.tm_min,

112 + tm.tm_sec, (long) tv.tv_usec, file, line);

113 /* align trace output (column 40 catches most files names in git) */

114 while (buf->len < 40)

115 strbuf_addch(buf, ' ');

120 -#endif

116

117 return 1;

118 }

@@ -229,74 +224,6 @@ static void trace_performance_vprintf_fl(const char *file, int line,

224 strbuf_release(&buf);

225 }

226

232 -#ifndef HAVE_VARIADIC_MACROS

233 -

234 -void trace_printf(const char *format, ...)

235 -{

236 - va_list ap;

237 - va_start(ap, format);

238 - trace_vprintf_fl(NULL, 0, &trace_default_key, format, ap);

239 - va_end(ap);

240 -}

241 -

242 -void trace_printf_key(struct trace_key *key, const char *format, ...)

243 -{

244 - va_list ap;

245 - va_start(ap, format);

246 - trace_vprintf_fl(NULL, 0, key, format, ap);

247 - va_end(ap);

248 -}

249 -

250 -void trace_argv_printf(const char **argv, const char *format, ...)

251 -{

252 - va_list ap;

253 - va_start(ap, format);

254 - trace_argv_vprintf_fl(NULL, 0, argv, format, ap);

255 - va_end(ap);

256 -}

257 -

258 -void trace_strbuf(struct trace_key *key, const struct strbuf *data)

259 -{

260 - trace_strbuf_fl(NULL, 0, key, data);

261 -}

262 -

263 -void trace_performance(uint64_t nanos, const char *format, ...)

264 -{

265 - va_list ap;

266 - va_start(ap, format);

267 - trace_performance_vprintf_fl(NULL, 0, nanos, format, ap);

268 - va_end(ap);

269 -}

270 -

271 -void trace_performance_since(uint64_t start, const char *format, ...)

272 -{

273 - va_list ap;

274 - va_start(ap, format);

275 - trace_performance_vprintf_fl(NULL, 0, getnanotime() - start,

276 - format, ap);

277 - va_end(ap);

278 -}

279 -

280 -void trace_performance_leave(const char *format, ...)

281 -{

282 - va_list ap;

283 - uint64_t since;

284 -

285 - if (perf_indent)

286 - perf_indent--;

287 -

288 - if (!format) /* Allow callers to leave without tracing anything */

289 - return;

290 -

291 - since = perf_start_times[perf_indent];

292 - va_start(ap, format);

293 - trace_performance_vprintf_fl(NULL, 0, getnanotime() - since,

294 - format, ap);

295 - va_end(ap);

296 -}

297 -

298 -#else

299 -

227 void trace_printf_key_fl(const char *file, int line, struct trace_key *key,

228 const char *format, ...)

229 {

@@ -342,9 +269,6 @@ void trace_performance_leave_fl(const char *file, int line,

269 va_end(ap);

270 }

271

345 -#endif /* HAVE_VARIADIC_MACROS */

346 -

347 -

272 static const char *quote_crnl(const char *path)

273 {

274 static struct strbuf new_path = STRBUF_INIT;

trace.h

+58 -70

@@ -126,71 +126,6 @@ void trace_command_performance(const char **argv);

126 void trace_verbatim(struct trace_key *key, const void *buf, unsigned len);

127 uint64_t trace_performance_enter(void);

128

129 -#ifndef HAVE_VARIADIC_MACROS

130 -

131 -/**

132 - * Prints a formatted message, similar to printf.

133 - */

134 -__attribute__((format (printf, 1, 2)))

135 -void trace_printf(const char *format, ...);

136 -

137 -__attribute__((format (printf, 2, 3)))

138 -void trace_printf_key(struct trace_key *key, const char *format, ...);

139 -

140 -/**

141 - * Prints a formatted message, followed by a quoted list of arguments.

142 - */

143 -__attribute__((format (printf, 2, 3)))

144 -void trace_argv_printf(const char **argv, const char *format, ...);

145 -

146 -/**

147 - * Prints the strbuf, without additional formatting (i.e. doesn't

148 - * choke on `%` or even `\0`).

149 - */

150 -void trace_strbuf(struct trace_key *key, const struct strbuf *data);

151 -

152 -/**

153 - * Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled.

154 - *

155 - * Example:

156 - * ------------

157 - * uint64_t t = 0;

158 - * for (;;) {

159 - * // ignore

160 - * t -= getnanotime();

161 - * // code section to measure

162 - * t += getnanotime();

163 - * // ignore

164 - * }

165 - * trace_performance(t, "frotz");

166 - * ------------

167 - */

168 -__attribute__((format (printf, 2, 3)))

169 -void trace_performance(uint64_t nanos, const char *format, ...);

170 -

171 -/**

172 - * Prints elapsed time since 'start' if GIT_TRACE_PERFORMANCE is enabled.

173 - *

174 - * Example:

175 - * ------------

176 - * uint64_t start = getnanotime();

177 - * // code section to measure

178 - * trace_performance_since(start, "foobar");

179 - * ------------

180 - */

181 -__attribute__((format (printf, 2, 3)))

182 -void trace_performance_since(uint64_t start, const char *format, ...);

183 -

184 -__attribute__((format (printf, 1, 2)))

185 -void trace_performance_leave(const char *format, ...);

186 -

187 -#else

188 -

189 -/*

190 - * Macros to add file:line - see above for C-style declarations of how these

191 - * should be used.

192 - */

193 -

129 /*

130 * TRACE_CONTEXT may be set to __FUNCTION__ if the compiler supports it. The

131 * default is __FILE__, as it is consistent with assert(), and static function

@@ -204,7 +139,10 @@ void trace_performance_leave(const char *format, ...);

139 # define TRACE_CONTEXT __FILE__

140 #endif

141

207 -/*

142 +/**

143 + * Macros to add the file:line of the calling code, instead of that of

144 + * the trace function itself.

145 + *

146 * Note: with C99 variadic macros, __VA_ARGS__ must include the last fixed

147 * parameter ('format' in this case). Otherwise, a call without variable

148 * arguments will have a surplus ','. E.g.:

@@ -220,6 +158,16 @@ void trace_performance_leave(const char *format, ...);

158 * comma, but this is non-standard.

159 */

160

161 +/**

162 + * trace_printf(), accepts "const char *format, ...".

163 + *

164 + * Prints a formatted message, similar to printf.

165 + */

166 +#define trace_printf(...) trace_printf_key(&trace_default_key, __VA_ARGS__)

167 +

168 +/**

169 + * trace_printf_key(), accepts "struct trace_key *key, const char *format, ...".

170 + */

171 #define trace_printf_key(key, ...) \

172 do { \

173 if (trace_pass_fl(key)) \

@@ -227,8 +175,11 @@ void trace_performance_leave(const char *format, ...);

175 __VA_ARGS__); \

176 } while (0)

177

230 -#define trace_printf(...) trace_printf_key(&trace_default_key, __VA_ARGS__)

231 -

178 +/**

179 + * trace_argv_printf(), accepts "struct trace_key *key, const char *format, ...)".

180 + *

181 + * Prints a formatted message, followed by a quoted list of arguments.

182 + */

183 #define trace_argv_printf(argv, ...) \

184 do { \

185 if (trace_pass_fl(&trace_default_key)) \

@@ -236,12 +187,36 @@ void trace_performance_leave(const char *format, ...);

187 argv, __VA_ARGS__); \

188 } while (0)

189

190 +/**

191 + * trace_strbuf(), accepts "struct trace_key *key, const struct strbuf *data".

192 + *

193 + * Prints the strbuf, without additional formatting (i.e. doesn't

194 + * choke on `%` or even `\0`).

195 + */

196 #define trace_strbuf(key, data) \

197 do { \

198 if (trace_pass_fl(key)) \

199 trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data);\

200 } while (0)

201

202 +/**

203 + * trace_performance(), accepts "uint64_t nanos, const char *format, ...".

204 + *

205 + * Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled.

206 + *

207 + * Example:

208 + * ------------

209 + * uint64_t t = 0;

210 + * for (;;) {

211 + * // ignore

212 + * t -= getnanotime();

213 + * // code section to measure

214 + * t += getnanotime();

215 + * // ignore

216 + * }

217 + * trace_performance(t, "frotz");

218 + * ------------

219 + */

220 #define trace_performance(nanos, ...) \

221 do { \

222 if (trace_pass_fl(&trace_perf_key)) \

@@ -249,6 +224,18 @@ void trace_performance_leave(const char *format, ...);

224 __VA_ARGS__); \

225 } while (0)

226

227 +/**

228 + * trace_performance_since(), accepts "uint64_t start, const char *format, ...".

229 + *

230 + * Prints elapsed time since 'start' if GIT_TRACE_PERFORMANCE is enabled.

231 + *

232 + * Example:

233 + * ------------

234 + * uint64_t start = getnanotime();

235 + * // code section to measure

236 + * trace_performance_since(start, "foobar");

237 + * ------------

238 + */

239 #define trace_performance_since(start, ...) \

240 do { \

241 if (trace_pass_fl(&trace_perf_key)) \

@@ -257,6 +244,9 @@ void trace_performance_leave(const char *format, ...);

244 __VA_ARGS__); \

245 } while (0)

246

247 +/**

248 + * trace_performance_leave(), accepts "const char *format, ...".

249 + */

250 #define trace_performance_leave(...) \

251 do { \

252 if (trace_pass_fl(&trace_perf_key)) \

@@ -285,6 +275,4 @@ static inline int trace_pass_fl(struct trace_key *key)

275 return key->fd || !key->initialized;

276 }

277

288 -#endif /* HAVE_VARIADIC_MACROS */

289 -

278 #endif /* TRACE_H */

trace2.c

-39

@@ -641,20 +641,6 @@ void trace2_region_enter_printf_fl(const char *file, int line,

641 va_end(ap);

642 }

643

644 -#ifndef HAVE_VARIADIC_MACROS

645 -void trace2_region_enter_printf(const char *category, const char *label,

646 - const struct repository *repo, const char *fmt,

647 - ...)

648 -{

649 - va_list ap;

650 -

651 - va_start(ap, fmt);

652 - trace2_region_enter_printf_va_fl(NULL, 0, category, label, repo, fmt,

653 - ap);

654 - va_end(ap);

655 -}

656 -#endif

657 -

644 void trace2_region_leave_printf_va_fl(const char *file, int line,

645 const char *category, const char *label,

646 const struct repository *repo,

@@ -717,20 +703,6 @@ void trace2_region_leave_printf_fl(const char *file, int line,

703 va_end(ap);

704 }

705

720 -#ifndef HAVE_VARIADIC_MACROS

721 -void trace2_region_leave_printf(const char *category, const char *label,

722 - const struct repository *repo, const char *fmt,

723 - ...)

724 -{

725 - va_list ap;

726 -

727 - va_start(ap, fmt);

728 - trace2_region_leave_printf_va_fl(NULL, 0, category, label, repo, fmt,

729 - ap);

730 - va_end(ap);

731 -}

732 -#endif

733 -

706 void trace2_data_string_fl(const char *file, int line, const char *category,

707 const struct repository *repo, const char *key,

708 const char *value)

@@ -826,17 +798,6 @@ void trace2_printf_fl(const char *file, int line, const char *fmt, ...)

798 va_end(ap);

799 }

800

829 -#ifndef HAVE_VARIADIC_MACROS

830 -void trace2_printf(const char *fmt, ...)

831 -{

832 - va_list ap;

833 -

834 - va_start(ap, fmt);

835 - trace2_printf_va_fl(NULL, 0, fmt, ap);

836 - va_end(ap);

837 -}

838 -#endif

839 -

801 const char *trace2_session_id(void)

802 {

803 return tr2_sid_get();

trace2.h

-25

@@ -397,18 +397,9 @@ void trace2_region_enter_printf_fl(const char *file, int line,

397 const struct repository *repo,

398 const char *fmt, ...);

399

400 -#ifdef HAVE_VARIADIC_MACROS

400 #define trace2_region_enter_printf(category, label, repo, ...) \

401 trace2_region_enter_printf_fl(__FILE__, __LINE__, (category), (label), \

402 (repo), __VA_ARGS__)

404 -#else

405 -/* clang-format off */

406 -__attribute__((format (region_enter_printf, 4, 5)))

407 -void trace2_region_enter_printf(const char *category, const char *label,

408 - const struct repository *repo, const char *fmt,

409 - ...);

410 -/* clang-format on */

411 -#endif

403

404 /**

405 * Emit a 'region_leave' event for <category>.<label> with optional

@@ -442,18 +433,9 @@ void trace2_region_leave_printf_fl(const char *file, int line,

433 const struct repository *repo,

434 const char *fmt, ...);

435

445 -#ifdef HAVE_VARIADIC_MACROS

436 #define trace2_region_leave_printf(category, label, repo, ...) \

437 trace2_region_leave_printf_fl(__FILE__, __LINE__, (category), (label), \

438 (repo), __VA_ARGS__)

449 -#else

450 -/* clang-format off */

451 -__attribute__((format (region_leave_printf, 4, 5)))

452 -void trace2_region_leave_printf(const char *category, const char *label,

453 - const struct repository *repo, const char *fmt,

454 - ...);

455 -/* clang-format on */

456 -#endif

439

440 /**

441 * Emit a key-value pair 'data' event of the form <category>.<key> = <value>.

@@ -506,14 +488,7 @@ void trace2_printf_va_fl(const char *file, int line, const char *fmt,

488

489 void trace2_printf_fl(const char *file, int line, const char *fmt, ...);

490

509 -#ifdef HAVE_VARIADIC_MACROS

491 #define trace2_printf(...) trace2_printf_fl(__FILE__, __LINE__, __VA_ARGS__)

511 -#else

512 -/* clang-format off */

513 -__attribute__((format (printf, 1, 2)))

514 -void trace2_printf(const char *fmt, ...);

515 -/* clang-format on */

516 -#endif

492

493 /*

494 * Optional platform-specific code to dump information about the

usage.c

+1 -14

@@ -299,10 +299,7 @@ static NORETURN void BUG_vfl(const char *file, int line, const char *fmt, va_lis

299 va_copy(params_copy, params);

300

301 /* truncation via snprintf is OK here */

302 - if (file)

303 - snprintf(prefix, sizeof(prefix), "BUG: %s:%d: ", file, line);

304 - else

305 - snprintf(prefix, sizeof(prefix), "BUG: ");

302 + snprintf(prefix, sizeof(prefix), "BUG: %s:%d: ", file, line);

303

304 vreportf(prefix, fmt, params);

305

@@ -317,7 +314,6 @@ static NORETURN void BUG_vfl(const char *file, int line, const char *fmt, va_lis

314 abort();

315 }

316

320 -#ifdef HAVE_VARIADIC_MACROS

317 NORETURN void BUG_fl(const char *file, int line, const char *fmt, ...)

318 {

319 va_list ap;

@@ -325,15 +321,6 @@ NORETURN void BUG_fl(const char *file, int line, const char *fmt, ...)

321 BUG_vfl(file, line, fmt, ap);

322 va_end(ap);

323 }

328 -#else

329 -NORETURN void BUG(const char *fmt, ...)

330 -{

331 - va_list ap;

332 - va_start(ap, fmt);

333 - BUG_vfl(NULL, 0, fmt, ap);

334 - va_end(ap);

335 -}

336 -#endif

324

325 #ifdef SUPPRESS_ANNOTATED_LEAKS

326 void unleak_memory(const void *ptr, size_t len)