From: Jani Nikula <jani@nikula.org>
To: Blake Jones <blakej@foo.net>
Subject: Re: [PATCH 10/10] timegm: add portable implementation (Solaris
	support)
In-Reply-To: <12349.1352043650@foo.net>
References: <12349.1352043650@foo.net>
User-Agent: Notmuch/0.14+81~g1924356 (http://notmuchmail.org) Emacs/23.4.1
	(i686-pc-linux-gnu)
Date: Sun, 04 Nov 2012 22:58:26 +0200
Message-ID: <87ip9lf5l9.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: notmuch@notmuchmail.org
Precedence: list

On Sun, 04 Nov 2012, Blake Jones <blakej@foo.net> wrote:
> Hi Jani,
>
>> I'd prefer to use timegm() where available, and the suggested
>> alternative [1] elsewhere.
>> 
>> [1] http://www.kernel.org/doc/man-pages/online/pages/man3/timegm.3.html
>
> I considered this alternative, but decided against it because it's
> completely MT-unsafe.  I don't know whether libnotmuch itself is
> MT-safe, but a process which called this routine in one thread would
> temporarily throw off any timezone-related work that any other threads
> were doing, even if they weren't using libnotmuch.

That is a valid point. Yet it doesn't change the fact that I'd prefer to
use timegm() where available. Internally, glibc uses the same code to
implement both timegm() and mktime(), and I'd hate it if the results
were subtly different depending on whether the time zone was specified
in the input or not. That said, I'm not opposed to using your simple
timegm() alternative in the compat code if you think it's good enough to
get you going on Solaris.

As to solving the compat linking problem, I think the patch at the end
of this message should fix it. Please try that with the regular notmuch
approach to portability. The general idea is to keep parse-time-string
as independent as possible from the rest of notmuch (possibly turning it
into a dynamic library and a package of its own eventually), but I think
including compat.h is an acceptable exception to make.

HTH,
Jani.


diff --git a/parse-time-string/Makefile.local b/parse-time-string/Makefile.local
index 53534f3..b3e5385 100644
--- a/parse-time-string/Makefile.local
+++ b/parse-time-string/Makefile.local
@@ -1,7 +1,9 @@
 dir := parse-time-string
 extra_cflags += -I$(srcdir)/$(dir)
 
-libparse-time-string_c_srcs := $(dir)/parse-time-string.c
+libparse-time-string_c_srcs =		\
+	$(notmuch_compat_srcs)		\
+	$(dir)/parse-time-string.c
 
 libparse-time-string_modules := $(libparse-time-string_c_srcs:.c=.o)
 
diff --git a/parse-time-string/parse-time-string.c b/parse-time-string/parse-time-string.c
index 584067d3..ccad422 100644
--- a/parse-time-string/parse-time-string.c
+++ b/parse-time-string/parse-time-string.c
@@ -32,6 +32,7 @@
 #include <sys/time.h>
 #include <sys/types.h>
 
+#include "compat.h"
 #include "parse-time-string.h"
 
 /*