From: Jani Nikula Date: Sun, 4 Nov 2012 10:21:01 +0000 (+0200) Subject: Re: [PATCH 10/10] timegm: add portable implementation (Solaris support) X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=84d1f6bea9f4e9d6f7e11405c3ba6d3e3f6b59b5;p=notmuch-archives.git Re: [PATCH 10/10] timegm: add portable implementation (Solaris support) --- diff --git a/38/29cad55131f2a44bb4edb8a6b86bbddfce766f b/38/29cad55131f2a44bb4edb8a6b86bbddfce766f new file mode 100644 index 000000000..a1a7d300e --- /dev/null +++ b/38/29cad55131f2a44bb4edb8a6b86bbddfce766f @@ -0,0 +1,302 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 0253A431FAF + for ; Sun, 4 Nov 2012 02:21:04 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.699 +X-Spam-Level: +X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5 + tests=[HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id pmrY5ULl70g2 for ; + Sun, 4 Nov 2012 02:21:03 -0800 (PST) +Received: from mail-oa0-f53.google.com (mail-oa0-f53.google.com + [209.85.219.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 046CD431FAE + for ; Sun, 4 Nov 2012 02:21:02 -0800 (PST) +Received: by mail-oa0-f53.google.com with SMTP id j6so5171694oag.26 + for ; Sun, 04 Nov 2012 02:21:02 -0800 (PST) +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=google.com; s=20120113; + h=mime-version:in-reply-to:references:date:message-id:subject:from:to + :cc:content-type:x-gm-message-state; + bh=zXSpgS3DMSzKlCOsj6qCbUDqK64q4wfMITJmEamc3zU=; + b=epCEiDFpj5uOPRlHcg3XUPubgEI8yDb0b9jNpF3mBg6LNowsC5sPB2Wwaouwrw7Y8w + lEjlm4N+Aic3G2zYkBCYtlVXreNQXT59jr7JJeBEGQoSib4DfcTLN+TPLi72e8ybx3/O + XcEEjI8pntMCCFmS3E5ZYH9NIQJ/vOugAEas8Tof6cCwoQEN5aD4iz5UDpUcArL7pNJD + EuXb8TOmKkB3acEM/N+U9WJkwiSaDPryMRZe7Mt6aq94YdTZMRNOCiJR96P7W2N+RMgD + gbkZfNsXiQ0WKfBi/XNFmyPmKGjRxuKlcmgBiSzJyQZzhiwoRciGgSRp7g9Yo1pkwIMT + Dc7A== +MIME-Version: 1.0 +Received: by 10.60.19.168 with SMTP id g8mr5235227oee.101.1352024461364; Sun, + 04 Nov 2012 02:21:01 -0800 (PST) +Received: by 10.76.69.138 with HTTP; Sun, 4 Nov 2012 02:21:01 -0800 (PST) +Received: by 10.76.69.138 with HTTP; Sun, 4 Nov 2012 02:21:01 -0800 (PST) +In-Reply-To: <1351998962-25135-11-git-send-email-blakej@foo.net> +References: <1351998962-25135-1-git-send-email-blakej@foo.net> + <1351998962-25135-11-git-send-email-blakej@foo.net> +Date: Sun, 4 Nov 2012 12:21:01 +0200 +Message-ID: + +Subject: Re: [PATCH 10/10] timegm: add portable implementation (Solaris + support) +From: Jani Nikula +To: Blake Jones +Content-Type: multipart/alternative; boundary=e89a8fb2062cbf4fcf04cda8b931 +X-Gm-Message-State: + ALoCoQn8vvIW7D7MKT4K4oUEILFZhdWQoLdHoC0rzjt/+cpov8pWrU3mjywLJxzGQbVepmLRh9xd +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Sun, 04 Nov 2012 10:21:04 -0000 + +--e89a8fb2062cbf4fcf04cda8b931 +Content-Type: text/plain; charset=UTF-8 + +On Nov 4, 2012 11:30 AM, "Blake Jones" wrote: +> +> The timegm(3) function is a non-standard extension to libc which is +> available in GNU libc and on some BSDs. Although SunOS had this +> function in its libc, Solaris (unfortunately) removed it. This patch +> implements a very simple version of timegm() which is good enough for +> parse-time-string.c. +> +> Although notmuch's idiom for portability is to test for native +> availability and put alternate versions in compat/, that approach led to +> a compilation problem in this case. libnotmuch.a includes a call to +> parse_time_string() from parse-time-vrp.o, and parse_time_string() in +> libparse-time-string.a needs to call timegm(). An attempt to create +> compat/timegm.c caused the link to fail, because libparse-time-string.a +> acquired a dependency on the new timegm.o in libnotmuch.a, and the +> linker only does a single pass on each ".a" looking for dependencies. +> This seems to be the case both for the GNU linker and the Solaris +> linker. A different possible workaround would have been to include +> libnotmuch.a multiple times on the link line, but that seemed like a +> brittle way to track this dependency. + +I'd prefer to use timegm() where available, and the suggested alternative +[1] elsewhere. I'll look into the compat build issues when I have a moment. + +Jani. + +[1] http://www.kernel.org/doc/man-pages/online/pages/man3/timegm.3.html + +> --- +> parse-time-string/parse-time-string.c | 37 +++++++++++++++++++++++++++++++++- +> 1 file changed, 36 insertions(+), 1 deletion(-) +> +> diff --git a/parse-time-string/parse-time-string.c +b/parse-time-string/parse-time-string.c +> index 584067d..28901af 100644 +> --- a/parse-time-string/parse-time-string.c +> +++ b/parse-time-string/parse-time-string.c +> @@ -1315,6 +1315,41 @@ fixup_ampm (struct state *state) +> return 0; +> } +> +> +static int +> +leapyear (int year) +> +{ +> + return ((year % 4) == 0 && ((year % 100) != 0 || (year % 400) == 0)); +> +} +> + +> +/* +> + * This is a simple implementation of timegm() which does what is needed +> + * by create_output() -- just turns the "struct tm" into a GMT time_t. +> + * It does not normalize any of the fields of the "struct tm", nor does +> + * it set tm_wday or tm_yday. +> + */ +> +static time_t +> +local_timegm (struct tm *tm) +> +{ +> + int monthlen[2][12] = { +> + { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }, +> + { 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }, +> + }; +> + int year, month, days; +> + +> + days = 365 * (tm->tm_year - 70); +> + for (year = 70; year < tm->tm_year; year++) { +> + if (leapyear(1900 + year)) { +> + days++; +> + } +> + } +> + for (month = 0; month < tm->tm_mon; month++) { +> + days += monthlen[leapyear(1900 + year)][month]; +> + } +> + days += tm->tm_mday - 1; +> + +> + return ((((days * 24) + tm->tm_hour) * 60 + tm->tm_min) * 60 + +tm->tm_sec); +> +} +> + +> /* Combine absolute and relative fields, and round. */ +> static int +> create_output (struct state *state, time_t *t_out, const time_t *ref, +> @@ -1465,7 +1500,7 @@ create_output (struct state *state, time_t *t_out, +const time_t *ref, +> if (is_field_set (state, TM_TZ)) { +> /* tm is in specified TZ, convert to UTC for timegm(3). */ +> tm.tm_min -= get_field (state, TM_TZ); +> - t = timegm (&tm); +> + t = local_timegm (&tm); +> } else { +> /* tm is in local time. */ +> t = mktime (&tm); +> -- +> 1.7.9.2 +> +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> http://notmuchmail.org/mailman/listinfo/notmuch + +--e89a8fb2062cbf4fcf04cda8b931 +Content-Type: text/html; charset=UTF-8 +Content-Transfer-Encoding: quoted-printable + +


+On Nov 4, 2012 11:30 AM, "Blake Jones" <blakej@foo.net> wrote:
+>
+> The timegm(3) function is a non-standard extension to libc which is +> available in GNU libc and on some BSDs. =C2=A0Although SunOS had this<= +br> +> function in its libc, Solaris (unfortunately) removed it. =C2=A0This p= +atch
+> implements a very simple version of timegm() which is good enough for<= +br> +> parse-time-string.c.
+>
+> Although notmuch's idiom for portability is to test for native
+> availability and put alternate versions in compat/, that approach led = +to
+> a compilation problem in this case. =C2=A0libnotmuch.a includes a call= + to
+> parse_time_string() from parse-time-vrp.o, and parse_time_string() in<= +br> +> libparse-time-string.a needs to call timegm(). =C2=A0An attempt to cre= +ate
+> compat/timegm.c caused the link to fail, because libparse-time-string.= +a
+> acquired a dependency on the new timegm.o in libnotmuch.a, and the
+> linker only does a single pass on each ".a" looking for depe= +ndencies.
+> This seems to be the case both for the GNU linker and the Solaris
+> linker. =C2=A0A different possible workaround would have been to inclu= +de
+> libnotmuch.a multiple times on the link line, but that seemed like a +> brittle way to track this dependency.

+

I'd prefer to use timegm() where available, and the suggested altern= +ative [1] elsewhere. I'll look into the compat build issues when I have= + a moment.

+

Jani.

+

[1] http://www.kernel.org/doc/man-pages/online/pages/man3/timegm.3.= +html

+

> ---
+> =C2=A0parse-time-string/parse-time-string.c | =C2=A0 37 ++++++++++++++= +++++++++++++++++++-
+> =C2=A01 file changed, 36 insertions(+), 1 deletion(-)
+>
+> diff --git a/parse-time-string/parse-time-string.c b/parse-time-string= +/parse-time-string.c
+> index 584067d..28901af 100644
+> --- a/parse-time-string/parse-time-string.c
+> +++ b/parse-time-string/parse-time-string.c
+> @@ -1315,6 +1315,41 @@ fixup_ampm (struct state *state)
+> =C2=A0 =C2=A0 =C2=A0return 0;
+> =C2=A0}
+>
+> +static int
+> +leapyear (int year)
+> +{
+> + =C2=A0 =C2=A0return ((year % 4) =3D=3D 0 && ((year % 100) != +=3D 0 || (year % 400) =3D=3D 0));
+> +}
+> +
+> +/*
+> + * This is a simple implementation of timegm() which does what is nee= +ded
+> + * by create_output() -- just turns the "struct tm" into a = +GMT time_t.
+> + * It does not normalize any of the fields of the "struct tm&quo= +t;, nor does
+> + * it set tm_wday or tm_yday.
+> + */
+> +static time_t
+> +local_timegm (struct tm *tm)
+> +{
+> + =C2=A0 =C2=A0int =C2=A0 =C2=A0 =C2=A0 =C2=A0monthlen[2][12] =3D { +> + =C2=A0 =C2=A0 =C2=A0 { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 3= +1 },
+> + =C2=A0 =C2=A0 =C2=A0 { 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 3= +1 },
+> + =C2=A0 =C2=A0};
+> + =C2=A0 =C2=A0int =C2=A0 =C2=A0 =C2=A0 =C2=A0year, month, days;
+> +
+> + =C2=A0 =C2=A0days =3D 365 * (tm->tm_year - 70);
+> + =C2=A0 =C2=A0for (year =3D 70; year < tm->tm_year; year++) { +> + =C2=A0 =C2=A0 =C2=A0 if (leapyear(1900 + year)) {
+> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 days++;
+> + =C2=A0 =C2=A0 =C2=A0 }
+> + =C2=A0 =C2=A0}
+> + =C2=A0 =C2=A0for (month =3D 0; month < tm->tm_mon; month++) {<= +br> +> + =C2=A0 =C2=A0 =C2=A0 days +=3D monthlen[leapyear(1900 + year)][month= +];
+> + =C2=A0 =C2=A0}
+> + =C2=A0 =C2=A0days +=3D tm->tm_mday - 1;
+> +
+> + =C2=A0 =C2=A0return ((((days * 24) + tm->tm_hour) * 60 + tm->t= +m_min) * 60 + tm->tm_sec);
+> +}
+> +
+> =C2=A0/* Combine absolute and relative fields, and round. */
+> =C2=A0static int
+> =C2=A0create_output (struct state *state, time_t *t_out, const time_t = +*ref,
+> @@ -1465,7 +1500,7 @@ create_output (struct state *state, time_t *t_ou= +t, const time_t *ref,
+> =C2=A0 =C2=A0 =C2=A0if (is_field_set (state, TM_TZ)) {
+> =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* tm is in specified TZ, convert to UTC f= +or timegm(3). */
+> =C2=A0 =C2=A0 =C2=A0 =C2=A0 tm.tm_min -=3D get_field (state, TM_TZ); +> - =C2=A0 =C2=A0 =C2=A0 t =3D timegm (&tm);
+> + =C2=A0 =C2=A0 =C2=A0 t =3D local_timegm (&tm);
+> =C2=A0 =C2=A0 =C2=A0} else {
+> =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* tm is in local time. */
+> =C2=A0 =C2=A0 =C2=A0 =C2=A0 t =3D mktime (&tm);
+> --
+> 1.7.9.2
+>
+> _______________________________________________
+> notmuch mailing list
+> notmuch@notmuchmail.org= +
+> http://not= +muchmail.org/mailman/listinfo/notmuch
+

+ +--e89a8fb2062cbf4fcf04cda8b931--