Re: [PATCH 1/2] date: recognize bogus FreeBSD gmtime output
- From
Jeff King <peff@peff.net>
- Date
- Apr 1, 2014, 21:28 UTC
- Message-ID
- <20140401212842.GB23578@sigill.intra.peff.net>
- In-Reply-To
- <533B2CDA.30307@web.de>
On Tue, Apr 01, 2014 at 11:17:14PM +0200, René Scharfe wrote:
Show 10 quoted lines
> >So are you saying we should set EOVERFLOW ourselves, or does FreeBSD > >set EOVERFLOW for us in this case and we do not have to worry? > > If we correct the return value then we should correct errno as well. > gmtime() on FreeBSD 10 leaves errno untouched when it encounters an > overflow. > > While testing this again I just noticed that FreeBSD doesn't strictly return > a pointer to a cleared struct tm. It simply leaves its static buffer > untouched. We should probably clear it (memset in git_gmtime_r).
Thanks, that all makes sense (we do not ever care about gmtime's errno in our code, but it does not hurt to be thorough to avoid surprising any new callers). Here's a replacement for patch 1.
-- >8 -- Subject: date: recognize bogus FreeBSD gmtime output
Most gmtime implementations return a NULL value when they encounter an error (and this behavior is specified by ANSI C and POSIX). FreeBSD's implementation, however, will simply leave the "struct tm" untouched. Let's also recognize this and convert it to a NULL (with this patch, t4212 should pass on FreeBSD).
Reported-by: René Scharfe <l.s.r@web.de> Signed-off-by: Jeff King <peff@peff.net> --- Makefile | 8 ++++++++ compat/gmtime.c | 29 +++++++++++++++++++++++++++++ config.mak.uname | 1 + git-compat-util.h | 7 +++++++ 4 files changed, 45 insertions(+) create mode 100644 compat/gmtime.c
diff --git a/Makefile b/Makefile index 3646391..2f3758c 100644 --- a/Makefile +++ b/Makefile @@ -338,6 +338,9 @@ all:: # Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite # with a different indexfile format version. If it isn't set the index # file format used is index-v[23]. +# +# Define GMTIME_UNRELIABLE_ERRORS if your gmtime() function does not +# return NULL when it receives a bogus time_t. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1489,6 +1492,11 @@ ifneq (,$(XDL_FAST_HASH)) BASIC_CFLAGS += -DXDL_FAST_HASH endif +ifdef GMTIME_UNRELIABLE_ERRORS + COMPAT_OBJS += compat/gmtime.o + BASIC_CFLAGS += -DGMTIME_UNRELIABLE_ERRORS +endif + ifeq ($(TCLTK_PATH),) NO_TCLTK = NoThanks endif diff --git a/compat/gmtime.c b/compat/gmtime.c new file mode 100644 index 0000000..e8362dd --- /dev/null +++ b/compat/gmtime.c @@ -0,0 +1,29 @@ +#include "../git-compat-util.h" +#undef gmtime +#undef gmtime_r + +struct tm *git_gmtime(const time_t *timep) +{ + static struct tm result; + return git_gmtime_r(timep, &result); +} + +struct tm *git_gmtime_r(const time_t *timep, struct tm *result) +{ + struct tm *ret; + + memset(result, 0, sizeof(*result)); + ret = gmtime_r(timep, result); + + /* + * Rather than NULL, FreeBSD gmtime simply leaves the "struct tm" + * untouched when it encounters overflow. Since "mday" cannot otherwise + * be zero, we can test this very quickly. + */ + if (ret && !ret->tm_mday) { + ret = NULL; + errno = EOVERFLOW; + } + + return ret; +} diff --git a/config.mak.uname b/config.mak.uname index 6069a44..0e22ac0 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -187,6 +187,7 @@ ifeq ($(uname_S),FreeBSD) endif PYTHON_PATH = /usr/local/bin/python HAVE_PATHS_H = YesPlease + GMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes endif ifeq ($(uname_S),OpenBSD) NO_STRCASESTR = YesPlease diff --git a/git-compat-util.h b/git-compat-util.h index 892032b..5191866 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -716,4 +716,11 @@ void warn_on_inaccessible(const char *path); /* Get the passwd entry for the UID of the current process. */ struct passwd *xgetpwuid_self(void); +#ifdef GMTIME_UNRELIABLE_ERRORS +struct tm *git_gmtime(const time_t *); +struct tm *git_gmtime_r(const time_t *, struct tm *); +#define gmtime git_gmtime +#define gmtime_r git_gmtime_r +#endif + #endif
-- 1.9.1.656.ge8a0637