From: Tomi Ollila <tomi.ollila@iki.fi>
To: Vladimir.Marek@oracle.com, notmuch@notmuchmail.org
Subject: Re: [PATCH 4/4] timegm: add portable implementation (Solaris support)
In-Reply-To: <1376663897-24385-5-git-send-email-Vladimir.Marek@oracle.com>
References: <1376663897-24385-1-git-send-email-Vladimir.Marek@oracle.com>
	<1376663897-24385-5-git-send-email-Vladimir.Marek@oracle.com>
User-Agent: Notmuch/0.16+3~g340c058 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Sat, 17 Aug 2013 09:23:53 +0300
Message-ID: <m2eh9sn8cm.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Cc: Vladimir Marek <vlmarek@volny.cz>
Precedence: list

Hi Vladimir, Blake and everyone else.

This patch set looks good to me and tests (in Linux) pass. 

>From code point of view these patches are IMO pushable.

There is just a small (but not so minor) thing missing;

The timegm(.c) implementation lacks copyright & licensing
information -- all the other compat files that are compiled
into executable does have this information so it is best
to keep this component also in line with those.

If you (either of you) can send new patch 4/4 as a reply to
this message it can be tracked along the first 3 in nmbug (*)

http://nmbug.tethera.net/status/

Tomi

On Fri, Aug 16 2013, Vladimir.Marek@oracle.com wrote:

> From: Blake Jones <blakej@foo.net>
>
> 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.
>
> Signed-off-by: Vladimir Marek <vlmarek@volny.cz>
> ---
>  compat/Makefile.local                 |    4 ++++
>  compat/compat.h                       |    5 +++++
>  compat/have_timegm.c                  |    7 +++++++
>  compat/timegm.c                       |   37 +++++++++++++++++++++++++++++++++
>  configure                             |   11 ++++++++++
>  parse-time-string/parse-time-string.c |    1 +
>  6 files changed, 65 insertions(+)
>  create mode 100644 compat/have_timegm.c
>  create mode 100644 compat/timegm.c
>
> diff --git a/compat/Makefile.local b/compat/Makefile.local
> index 2c4f65f..b0d5417 100644
> --- a/compat/Makefile.local
> +++ b/compat/Makefile.local
> @@ -17,4 +17,8 @@ ifneq ($(HAVE_STRSEP),1)
>  notmuch_compat_srcs += $(dir)/strsep.c
>  endif
>  
> +ifneq ($(HAVE_TIMEGM),1)
> +notmuch_compat_srcs += $(dir)/timegm.c
> +endif
> +
>  SRCS := $(SRCS) $(notmuch_compat_srcs)
> diff --git a/compat/compat.h b/compat/compat.h
> index ae762c3..5a402d5 100644
> --- a/compat/compat.h
> +++ b/compat/compat.h
> @@ -57,6 +57,11 @@ char* strcasestr(const char *haystack, const char *needle);
>  char *strsep(char **stringp, const char *delim);
>  #endif /* !HAVE_STRSEP */
>  
> +#if !HAVE_TIMEGM
> +#include <time.h>
> +time_t timegm (struct tm *tm);
> +#endif /* !HAVE_TIMEGM */
> +
>  /* Silence gcc warnings about unused results.  These warnings exist
>   * for a reason; any use of this needs to be justified. */
>  #ifdef __GNUC__
> diff --git a/compat/have_timegm.c b/compat/have_timegm.c
> new file mode 100644
> index 0000000..b62b793
> --- /dev/null
> +++ b/compat/have_timegm.c
> @@ -0,0 +1,7 @@
> +#include <time.h>
> +#include "compat.h"
> +
> +int main()
> +{
> +    return (int) timegm((struct tm *)0);
> +}
> diff --git a/compat/timegm.c b/compat/timegm.c
> new file mode 100644
> index 0000000..6d76164
> --- /dev/null
> +++ b/compat/timegm.c
> @@ -0,0 +1,37 @@
> +#include <time.h>
> +#include "compat.h"
> +
> +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.
> + */
> +time_t
> +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);
> +}
> diff --git a/configure b/configure
> index ac44857..6166917 100755
> --- a/configure
> +++ b/configure
> @@ -530,6 +530,17 @@ else
>  fi
>  rm -f compat/have_strsep
>  
> +printf "Checking for timegm... "
> +if ${CC} -o compat/have_timegm "$srcdir"/compat/have_timegm.c > /dev/null 2>&1
> +then
> +    printf "Yes.\n"
> +    have_timegm="1"
> +else
> +    printf "No (will use our own instead).\n"
> +    have_timegm="0"
> +fi
> +rm -f compat/have_timegm
> +
>  printf "Checking for standard version of getpwuid_r... "
>  if ${CC} -o compat/check_getpwuid "$srcdir"/compat/check_getpwuid.c > /dev/null 2>&1
>  then
> diff --git a/parse-time-string/parse-time-string.c b/parse-time-string/parse-time-string.c
> index 584067d..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"
>  
>  /*
> -- 
> 1.7.9.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch