To: Tomi Ollila <tomi.ollila@iki.fi>
Subject: Re: [PATCH 10/10] timegm: add portable implementation (Solaris
	support) 
In-Reply-To: Your message of "Mon, 05 Nov 2012 19:36:47 +0200."
	<m2a9uwyms0.fsf@guru.guru-group.fi> 
Date: Mon, 05 Nov 2012 10:33:12 -0800
Message-ID: <10299.1352140392@foo.net>
From: Blake Jones <blakej@foo.net>
Cc: notmuch@notmuchmail.org
Precedence: list

>> The other approaches rely on letting libc do all the hard work of
>> time zone manipulation, and then reading the tea leaves to find a way
>> to undo it.
> 
> Did you look at the gnu libc version -- I bet it is pretty hairy...

I didn't look at either the GNU or the Solaris libc version.  But the
file that implements the timezone handling (and localtime(), mktime(),
etc.) in Solaris' libc is nearly 3000 lines of code, so I suspect
there's an awful lot of stuff going on.

>> For what it's worth, I used the attached program to test my
>> implementation, and it passed.
> 
> Thanks, It's nice to see your simple implementation passes these
> tests...
> 
> Just for curiosity: What do you think lacks in your timegm() that it
> could not be promoted as 'complete timegm() solution'.

Well, since there isn't a standard for timegm(), I'm comparing it to
what glibc and BSD do.  The glibc mktime() man page, for example,
mentions that mktime() modifies the fields of the tm structure as
follows:

    - tm_wday and tm_yday are set to values determined from the contents
      of the other fields

    - if structure members are outside their valid interval, they will
      be normalized (so that, for example, 40 October is changed into 9
      November)

    - tm_isdst is set (regardless of its initial value) to a positive
      value or to 0, respectively, to indicate whether DST is or is not
      in effect at the specified time.

    - Calling mktime() also sets the external variable tzname with
      information about the current timezone. 

The corresponding timegm() man page for glibc doesn't say whether
timegm() does the same thing, but I would assume it does.  The FreeBSD
timegm() man page says that its version does update the fields of the
"tm" structure, like its mktime() implementation.

My implementation of timegm() does none of these things.  It treats the
passed-in "struct tm" as constant, and just returns a valid time_t.  If
you really wanted to make this mktime() be as capable as the ones in the
GNU and BSD libc's, you could have it turn around and call gmtime_r() on
the generated time_t, and pass the original "struct tm" to gmtime_r().
Personally, I think this is unnecessary overloading of a function that
does this one thing just fine, and in practice Jani's code doesn't seem
to need it, so I didn't bother.

> In any way, including this timegm() function in compat suits me fine.

Great.

Blake