{"thread":{"id":"53470","subject":"[PATCH] compat: remove gmtime","startedAt":"2020-05-14T19:19:10Z","lastAt":"2020-05-14T22:12:05Z","messageCount":2,"participants":["Carlo Marcelo Arenas Belón","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"397819","messageId":"20200514191854.55231-1-carenas@gmail.com","threadId":"53470","inReplyTo":null,"subject":"[PATCH] compat: remove gmtime","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2020-05-14T19:18:54Z","receivedAt":"2020-05-14T19:19:10Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"ccd469450a (date.c: switch to reentrant {gm,local}time_r, 2019-11-28)\nremoves the only gmtime() call we had and moves to gmtime_r() which\ndoesn't have the same portability problems.\n\nRemove the compat gmtime code since it is no longer needed, and confirm\nby successfull running t4212 in FreeBSD 9.3 amd64 (the oldest I could\nget a hold off).\n\nFurther work might be needed to ensure 32bit time_t systems (like FreeBSD\ni386) will handle correctly the overflows tested in t4212, but that is\northogonal to this change, and it doesn't change the current behaviour\nas neither gmtime() or gmtime_r() will ever return NULL on those systems\nbecause time_t is unsigned.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n Makefile          |  8 --------\n compat/gmtime.c   | 29 -----------------------------\n config.mak.uname  |  1 -\n git-compat-util.h |  7 -------\n 4 files changed, 45 deletions(-)\n delete mode 100644 compat/gmtime.c\n\ndiff --git a/Makefile b/Makefile\nindex 9804a0758b..12c650f259 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -405,9 +405,6 @@ all::\n # with a different indexfile format version.  If it isn't set the index\n # file format used is index-v[23].\n #\n-# Define GMTIME_UNRELIABLE_ERRORS if your gmtime() function does not\n-# return NULL when it receives a bogus time_t.\n-#\n # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime.\n #\n # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC.\n@@ -1803,11 +1800,6 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n \tMSGFMT += --check --statistics\n endif\n \n-ifdef GMTIME_UNRELIABLE_ERRORS\n-\tCOMPAT_OBJS += compat/gmtime.o\n-\tBASIC_CFLAGS += -DGMTIME_UNRELIABLE_ERRORS\n-endif\n-\n ifdef HAVE_CLOCK_GETTIME\n \tBASIC_CFLAGS += -DHAVE_CLOCK_GETTIME\n endif\ndiff --git a/compat/gmtime.c b/compat/gmtime.c\ndeleted file mode 100644\nindex e8362dd2b9..0000000000\n--- a/compat/gmtime.c\n+++ /dev/null\n@@ -1,29 +0,0 @@\n-#include \"../git-compat-util.h\"\n-#undef gmtime\n-#undef gmtime_r\n-\n-struct tm *git_gmtime(const time_t *timep)\n-{\n-\tstatic struct tm result;\n-\treturn git_gmtime_r(timep, &result);\n-}\n-\n-struct tm *git_gmtime_r(const time_t *timep, struct tm *result)\n-{\n-\tstruct tm *ret;\n-\n-\tmemset(result, 0, sizeof(*result));\n-\tret = gmtime_r(timep, result);\n-\n-\t/*\n-\t * Rather than NULL, FreeBSD gmtime simply leaves the \"struct tm\"\n-\t * untouched when it encounters overflow. Since \"mday\" cannot otherwise\n-\t * be zero, we can test this very quickly.\n-\t */\n-\tif (ret && !ret->tm_mday) {\n-\t\tret = NULL;\n-\t\terrno = EOVERFLOW;\n-\t}\n-\n-\treturn ret;\n-}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 2bff8b3466..c0852b37a5 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -244,7 +244,6 @@ ifeq ($(uname_S),FreeBSD)\n \tPYTHON_PATH = /usr/local/bin/python\n \tPERL_PATH = /usr/local/bin/perl\n \tHAVE_PATHS_H = YesPlease\n-\tGMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes\n \tHAVE_BSD_SYSCTL = YesPlease\n \tHAVE_BSD_KERN_PROC_SYSCTL = YesPlease\n \tPAGER_ENV = LESS=FRX LV=-c MORE=FRX\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex aed0b5d4f9..f1b0a5aa0a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1208,13 +1208,6 @@ int access_or_die(const char *path, int mode, unsigned flag);\n /* Warn on an inaccessible file if errno indicates this is an error */\n int warn_on_fopen_errors(const char *path);\n \n-#ifdef GMTIME_UNRELIABLE_ERRORS\n-struct tm *git_gmtime(const time_t *);\n-struct tm *git_gmtime_r(const time_t *, struct tm *);\n-#define gmtime git_gmtime\n-#define gmtime_r git_gmtime_r\n-#endif\n-\n #if !defined(USE_PARENS_AROUND_GETTEXT_N) && defined(__GNUC__)\n #define USE_PARENS_AROUND_GETTEXT_N 1\n #endif\n-- \n2.26.2.812.g046d49d455\n\n"},{"id":"397838","messageId":"20200514221202.GB3074610@coredump.intra.peff.net","threadId":"53470","inReplyTo":"20200514191854.55231-1-carenas@gmail.com","subject":"Re: [PATCH] compat: remove gmtime","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-14T22:12:02Z","receivedAt":"2020-05-14T22:12:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 14, 2020 at 12:18:54PM -0700, Carlo Marcelo Arenas Belón wrote:\n\n> ccd469450a (date.c: switch to reentrant {gm,local}time_r, 2019-11-28)\n> removes the only gmtime() call we had and moves to gmtime_r() which\n> doesn't have the same portability problems.\n> \n> Remove the compat gmtime code since it is no longer needed, and confirm\n> by successfull running t4212 in FreeBSD 9.3 amd64 (the oldest I could\n> get a hold off).\n\nSo gmtime_r() behaves differently than gmtime() on those systems in the\nface of an overflowing time_t? That seems like an odd implementation\nchoice, but OK. If the test isn't failing, I'm just as happy to get rid\nof this compat cruft.\n\n-Peff\n"}]}