unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Blake Jones <blakej@foo.net>
To: Tomi Ollila <tomi.ollila@iki.fi>
Cc: notmuch@notmuchmail.org
Subject: Re: [PATCH 10/10] timegm: add portable implementation (Solaris support) 
Date: Mon, 05 Nov 2012 10:33:12 -0800	[thread overview]
Message-ID: <10299.1352140392@foo.net> (raw)
In-Reply-To: Your message of "Mon, 05 Nov 2012 19:36:47 +0200." <m2a9uwyms0.fsf@guru.guru-group.fi>

>> 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

  reply	other threads:[~2012-11-05 18:33 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-04  3:15 [PATCH 00/10] Solaris support Blake Jones
2012-11-04  3:15 ` [PATCH 01/10] getpwuid: check for standards compliance (Solaris support) Blake Jones
2012-11-04  3:15 ` [PATCH 02/10] asctime: " Blake Jones
2012-11-04  3:15 ` [PATCH 03/10] gethostbyname: check for libnsl " Blake Jones
2012-11-04  3:15 ` [PATCH 04/10] configure: check for -Wl,-rpath " Blake Jones
2012-11-04  3:15 ` [PATCH 05/10] install: check for non-SysV version " Blake Jones
2012-11-04 21:31   ` Jani Nikula
2012-11-05  5:27     ` Blake Jones
2012-11-05 11:29       ` Jani Nikula
2012-11-05 14:52         ` Blake Jones
2012-11-04  3:15 ` [PATCH 06/10] strsep: check for availability " Blake Jones
2012-11-04  3:15 ` [PATCH 07/10] gen-version-script: parse Solaris "nm" output " Blake Jones
2012-11-04  3:16 ` [PATCH 08/10] notmuch-config: header for index() prototype " Blake Jones
2012-11-04 21:16   ` Jani Nikula
2012-11-04 21:47     ` Tomi Ollila
2012-11-05  4:52       ` Blake Jones
2012-11-04  3:16 ` [PATCH 09/10] debugger.c: correct return type from getppid() " Blake Jones
2012-11-04  3:16 ` [PATCH 10/10] timegm: add portable implementation " Blake Jones
2012-11-04 10:21   ` Jani Nikula
2012-11-04 15:40     ` Blake Jones
2012-11-04 20:58       ` Jani Nikula
2012-11-05  4:50         ` Blake Jones
2012-11-05 12:09       ` Tomi Ollila
2012-11-05 15:47         ` Blake Jones
2012-11-05 17:36           ` Tomi Ollila
2012-11-05 18:33             ` Blake Jones [this message]
2012-11-04 21:35 ` [PATCH 00/10] Solaris support Jani Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=10299.1352140392@foo.net \
    --to=blakej@foo.net \
    --cc=notmuch@notmuchmail.org \
    --cc=tomi.ollila@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).