On Mon, Nov 30, 2020 at 07:30:06PM -0500, Taylor Blau wrote: > @@ -147,7 +148,7 @@ int cmd_bugreport(int argc, const char **argv, const char *prefix) > strbuf_complete(&report_path, '/'); > > strbuf_addstr(&report_path, "git-bugreport-"); > - strbuf_addftime(&report_path, option_suffix, localtime(&now), 0, 0); > + strbuf_addftime(&report_path, option_suffix, localtime_r(&now, &tm), 0, 0); > strbuf_addstr(&report_path, ".txt"); I briefly wondered if we'd want a strbuf_addftime() variant that just takes a time_t. But the choice of localtime vs gmtime makes this awkward, not to mention the gymnastics we do in show_date() to get things into the author's zone. So this looks good to me. We might also want to do this on top: -- >8 -- Subject: [PATCH] banned.h: mark non-reentrant gmtime, etc as banned The traditional gmtime(), localtime(), ctime(), and asctime() functions return pointers to shared storage. This means they're not thread-safe, and they also run the risk of somebody holding onto the result across multiple calls (where each call invalidates the previous result). All callers should be using gmtime_r() or localtime_r() instead. The ctime_r() and asctime_r() functions are OK in that respect, but have no check that the buffer we pass in is long enough (the manpage says it "should have room for at least 26 bytes"). Since this is such an easy-to-get-wrong interface, and since we have the much safer stftime() as well as its more conveinent strbuf_addftime() wrapper, let's likewise ban both of those. Signed-off-by: Jeff King <peff@xxxxxxxx> --- TBH, ctime() and its variants are so awful that I doubt anybody would try to use them, but it doesn't hurt to err on the side of caution. banned.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/banned.h b/banned.h index 60a18d4403..7ab4f2e492 100644 --- a/banned.h +++ b/banned.h @@ -29,4 +29,17 @@ #define vsprintf(buf,fmt,arg) BANNED(vsprintf) #endif +#undef gmtime +#define gmtime(t) BANNED(gmtime) +#undef localtime +#define localtime(t) BANNED(localtime) +#undef ctime +#define ctime(t) BANNED(ctime) +#undef ctime_r +#define ctime_r(t, buf) BANNED(ctime_r) +#undef asctime +#define asctime(t) BANNED(asctime) +#undef asctime_r +#define asctime_r(t, buf) BANNED(asctime_r) + #endif /* BANNED_H */ -- 2.29.2.853.g04e16501f9
