[v2,1/2] banned.h: mark non-reentrant gmtime, etc as banned

Commit Message

Junio C Hamano Dec. 1, 2020, 9:11 p.m. UTC

From: Jeff King <[e-mail]>

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 their reentrant counterparts.

Signed-off-by: Jeff King <[e-mail]>
Signed-off-by: Junio C Hamano <[e-mail]>
---
 banned.h | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

SZEDER Gábor Dec. 6, 2020, 2:56 p.m. UTC | #1

On Tue, Dec 01, 2020 at 01:11:37PM -0800, Junio C Hamano wrote:
> From: Jeff King <[e-mail]>
> 
> 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 their reentrant counterparts.
> 
> Signed-off-by: Jeff King <[e-mail]>
> Signed-off-by: Junio C Hamano <[e-mail]>
> ---
>  banned.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/banned.h b/banned.h
> index 60a18d4403..ed11300bb2 100644
> --- a/banned.h
> +++ b/banned.h
> @@ -29,4 +29,13 @@
>  #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 asctime
> +#define asctime(t) BANNED(asctime)
> +
>  #endif /* BANNED_H */

This patch should be queued on top of topic
'tb/bugreport-no-localtime'.  Currently they are on parallel branches:

  * 91aef03015 (refs/upstream/jk/banned) banned.h: mark ctime_r() and asctime_r() as banned
  * 1fbfdf556f banned.h: mark non-reentrant gmtime, etc as banned
  | * 4f6460df55 (refs/upstream/tb/bugreport-no-localtime) builtin/bugreport.c: use thread-safe localtime_r()
  |/  
  * 72ffeb997e Ninth batch

and because of the not-yet-removed localtime() call in
'builtin/bugreport.c' commits 1fbfdf556f and 91aef03015 can't be
built.

Patch

diff --git a/banned.h b/banned.h
index 60a18d4403..ed11300bb2 100644
--- a/banned.h
+++ b/banned.h
@@ -29,4 +29,13 @@ 
 #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 asctime
+#define asctime(t) BANNED(asctime)
+
 #endif /* BANNED_H */