Re: [PATCH 1/2] date: recognize bogus FreeBSD gmtime output
Am 01.04.2014 09:42, schrieb Jeff King:
diff --git a/compat/gmtime.c b/compat/gmtime.c
new file mode 100644
index 000..ffcabf4
--- /dev/null
+++ b/compat/gmtime.c
@@ -0,0 +1,26 @@
+#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;
+
+ ret = gmtime_r(timep, result);
+
+ /*
+* Rather than NULL, FreeBSD gmtime will return a struct tm with all
+* fields zeroed. Since mday cannot otherwise be zero, we can test
+* this very quickly.
+*/
+ if (ret !ret-tm_mday)
+ ret = NULL;
+
+ return ret;
+}
http://pubs.opengroup.org/onlinepubs/009695399/functions/gmtime.html
says that errno shall be set on error and only mentions EOVERFLOW as a
possible error code.
René
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] date: recognize bogus FreeBSD gmtime output
René Scharfe l@web.de writes:
Am 01.04.2014 09:42, schrieb Jeff King:
diff --git a/compat/gmtime.c b/compat/gmtime.c
new file mode 100644
index 000..ffcabf4
--- /dev/null
+++ b/compat/gmtime.c
@@ -0,0 +1,26 @@
+#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;
+
+ret = gmtime_r(timep, result);
+
+/*
+ * Rather than NULL, FreeBSD gmtime will return a struct tm with all
+ * fields zeroed. Since mday cannot otherwise be zero, we can test
+ * this very quickly.
+ */
+if (ret !ret-tm_mday)
+ret = NULL;
+
+return ret;
+}
http://pubs.opengroup.org/onlinepubs/009695399/functions/gmtime.html
says that errno shall be set on error and only mentions EOVERFLOW as a
possible error code.
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?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] date: recognize bogus FreeBSD gmtime output
Am 01.04.2014 21:08, schrieb Junio C Hamano:
René Scharfe l@web.de writes:
Am 01.04.2014 09:42, schrieb Jeff King:
diff --git a/compat/gmtime.c b/compat/gmtime.c
new file mode 100644
index 000..ffcabf4
--- /dev/null
+++ b/compat/gmtime.c
@@ -0,0 +1,26 @@
+#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;
+
+ ret = gmtime_r(timep, result);
+
+ /*
+* Rather than NULL, FreeBSD gmtime will return a struct tm with all
+* fields zeroed. Since mday cannot otherwise be zero, we can test
+* this very quickly.
+*/
+ if (ret !ret-tm_mday)
+ ret = NULL;
+
+ return ret;
+}
http://pubs.opengroup.org/onlinepubs/009695399/functions/gmtime.html
says that errno shall be set on error and only mentions EOVERFLOW as a
possible error code.
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).
Demo program:
-- 8 --
#include stdio.h
#include time.h
const static time_t epoch;
int main(int argc, char **argv)
{
time_t t = 9;
printf(%s, ctime(t));
printf(%s, asctime(gmtime(t)));
printf(%s, ctime(t));
gmtime(epoch);
printf(%s, asctime(gmtime(t)));
return 0;
}
-- 8 --
Results on FreeBSD 10:
Wed Sep 6 11:46:39 -1126091476
Wed Sep 6 11:46:39 -1126091476
Wed Sep 6 11:46:39 -1126091476
Thu Jan 1 00:00:00 1970
René
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] date: recognize bogus FreeBSD gmtime output
On Tue, Apr 01, 2014 at 11:17:14PM +0200, René Scharfe wrote:
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@web.de
Signed-off-by: Jeff King p...@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 000..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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
