git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/3] Phase out non-reentrant time functions

From
Doan Tran Cong Danh <congdanhqx@gmail.com>
Date
Nov 28, 2019, 12:25 UTC
Message-ID
<cover.1574943677.git.congdanhqx@gmail.com>
In-Reply-To
<cover.1574867409.git.congdanhqx@gmail.com>
gmtime/localtime is considered unsafe in multithread environment.

git was started as single-thread application, but we have some multi-thread code, right now.

replace all usage of gmtime/localtime by their respective reentrant ones.

On most POSIX systems, we won't notice any differences in performance, since they implemented gmtime and gmtime_r mostly the same.

On Windows, we may be taxed, since gmtime_r and localtime_r is our compat functions, because we memcpy from returned data of gmtime/localtime.

To address that, I made patch #3 and included it together with this series. I'm not sure how much portable it is.

Doan Tran Cong Danh (3):
  date.c: switch to reentrant {gm,local}time_r
  archive-zip.c: switch to reentrant localtime_r
  mingw: use {gm,local}time_s as backend for {gm,local}time_r
 archive-zip.c  | 10 +++++-----
 compat/mingw.c | 12 ++++++------
 date.c         | 18 ++++++++++--------
 3 files changed, 21 insertions(+), 19 deletions(-)
Range-diff against v1:
1:  1e7e08b3c7 < -:  ---------- date.c::datestamp: switch to reentrant localtime_r
2:  130eba77db ! 1:  71de738864 date.c::time_to_tm_local: use reentrant localtime_r(3)
    @@ Metadata
     Author: Doan Tran Cong Danh <congdanhqx@gmail.com>
     
      ## Commit message ##
    -    date.c::time_to_tm_local: use reentrant localtime_r(3)
    +    date.c: switch to reentrant {gm,local}time_r
     
    -    Phase out `localtime(3)' in favour of reentrant `localtime_r(3)'
    +    Originally, git was intended to be single-thread executable.
    +    `gmtime(3)' and `localtime(3)' can be used in such codebase
    +    for cleaner code.
    +
    +    Overtime, we're employing multithread in our code base.
    +
    +    Let's phase out `gmtime(3)' and `localtime(3)' in favour of
    +    `gmtime_r(3)' and `localtime_r(3)'.
     
         Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>
     
      ## date.c ##
    -@@ date.c: static struct tm *time_to_tm(timestamp_t time, int tz)
    - 	return gmtime(&t);
    +@@ date.c: static time_t gm_time_t(timestamp_t time, int tz)
    +  * thing, which means that tz -0100 is passed in as the integer -100,
    +  * even though it means "sixty minutes off"
    +  */
    +-static struct tm *time_to_tm(timestamp_t time, int tz)
    ++static struct tm *time_to_tm(timestamp_t time, int tz, struct tm *tm)
    + {
    + 	time_t t = gm_time_t(time, tz);
    +-	return gmtime(&t);
    ++	return gmtime_r(&t, tm);
      }
      
     -static struct tm *time_to_tm_local(timestamp_t time)
    @@ date.c: const char *show_date(timestamp_t time, int tz, const struct date_mode *
     -		tm = time_to_tm_local(time);
     +		tm = time_to_tm_local(time, &tmbuf);
      	else
    - 		tm = time_to_tm(time, tz);
    +-		tm = time_to_tm(time, tz);
    ++		tm = time_to_tm(time, tz, &tmbuf);
      	if (!tm) {
    +-		tm = time_to_tm(0, 0);
    ++		tm = time_to_tm(0, 0, &tmbuf);
    + 		tz = 0;
    + 	}
    + 
    +@@ date.c: void datestamp(struct strbuf *out)
    + {
    + 	time_t now;
    + 	int offset;
    ++	struct tm tm = { 0 };
    + 
    + 	time(&now);
    + 
    +-	offset = tm_to_time_t(localtime(&now)) - now;
    ++	offset = tm_to_time_t(localtime_r(&now, &tm)) - now;
    + 	offset /= 60;
    + 
    + 	date_string(now, offset, out);
3:  ce7fe7bcf3 < -:  ---------- date.c::time_to_tm: use reentrant gmtime_r(3)
4:  f6fd89dfe1 ! 2:  77798f1773 archive-zip: use reentrant localtime_r(3)
    @@ Metadata
     Author: Doan Tran Cong Danh <congdanhqx@gmail.com>
     
      ## Commit message ##
    -    archive-zip: use reentrant localtime_r(3)
    +    archive-zip.c: switch to reentrant localtime_r
    +
    +    Originally, git was intended to be single-thread executable.
    +    `localtime(3)' can be used in such codebase for cleaner code.
    +
    +    Overtime, we're employing multithread in our code base.
    +
    +    Let's phase out `gmtime(3)' in favour of `localtime_r(3)'.
    +
    +    Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>
     
      ## archive-zip.c ##
     @@ archive-zip.c: static void write_zip_trailer(const struct object_id *oid)
5:  2c39f9a04f ! 3:  33a67eb377 mingw: use {gm,local}time_s as backend for {gm,local}time_r
    @@ Commit message
         Signed-off-by: Doan Tran Cong Danh <congdanhqx@gmail.com>
     
      ## compat/mingw.c ##
    -@@
    - #include "../git-compat-util.h"
    - #include "win32.h"
    - #include <conio.h>
    -+#include <errno.h>
    - #include <wchar.h>
    - #include "../strbuf.h"
    - #include "../run-command.h"
     @@ compat/mingw.c: int pipe(int filedes[2])
      
      struct tm *gmtime_r(const time_t *timep, struct tm *result)
-- 
2.24.0.615.g37f5bfbdea
Previous: Danh DoanNext: Doan Tran Cong Danh
Message 12 of 15 in “drop non-reentrant time usage”
  1. 0/5 drop non-reentrant time usageDoan Tran Cong Danh, Nov 27, 2019
  2. 1/5 date.c::datestamp: switch to reentrant localtime_rDoan Tran Cong Danh, Nov 27, 2019
  3. 2/5 date.c::time_to_tm_local: use reentrant localtime_r(3)Doan Tran Cong Danh, Nov 27, 2019
  4. 3/5 date.c::time_to_tm: use reentrant gmtime_r(3)Doan Tran Cong Danh, Nov 27, 2019
  5. 4/5 archive-zip: use reentrant localtime_r(3)Doan Tran Cong Danh, Nov 27, 2019
  6. 5/5 mingw: use {gm,local}time_s as backend for {gm,local}time_rDoan Tran Cong Danh, Nov 27, 2019
  7. Johannes SchindelinNov 27, 2019
  8. Johannes SchindelinNov 27, 2019
  9. Danh DoanNov 28, 2019
  10. Jeff KingNov 27, 2019
  11. Danh DoanNov 28, 2019
  12. 0/3 Phase out non-reentrant time functionsDoan Tran Cong Danh, Nov 28, 2019
  13. 1/3 date.c: switch to reentrant {gm,local}time_rDoan Tran Cong Danh, Nov 28, 2019
  14. 2/3 archive-zip.c: switch to reentrant localtime_rDoan Tran Cong Danh, Nov 28, 2019
  15. 3/3 mingw: use {gm,local}time_s as backend for {gm,local}time_rDoan Tran Cong Danh, Nov 28, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.