Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #66098 > unrolled thread

[code review] UTC-to-seconds

Back to article view | Back to comp.lang.c

Contents

  [code review] UTC-to-seconds Francois Grieu <fgrieu@gmail.com> - 2015-07-21 13:17 +0200
    Re: [code review] UTC-to-seconds Tim Rentsch <txr@alumni.caltech.edu> - 2015-07-24 09:31 -0700
      Re: [code review] UTC-to-seconds Keith Thompson <kst-u@mib.org> - 2015-07-24 10:36 -0700
        Re: [code review] UTC-to-seconds James Kuyper <jameskuyper@verizon.net> - 2015-07-24 13:54 -0400
          Re: [code review] UTC-to-seconds Keith Thompson <kst-u@mib.org> - 2015-07-24 12:21 -0700
            Re: [code review] UTC-to-seconds James Kuyper <jameskuyper@verizon.net> - 2015-07-24 15:59 -0400
        Re: [code review] UTC-to-seconds Tim Rentsch <txr@alumni.caltech.edu> - 2015-07-26 11:39 -0700
        Re: [code review] UTC-to-seconds Alain Ketterlin <alain@universite-de-strasbourg.fr.invalid> - 2015-07-26 22:58 +0200

#66098 — [code review] UTC-to-seconds

The following aims at converting from UTC date in  struct tm  format
to seconds, with portability in mind, including to systems with
16-bit int or/and lacking 64-bit and floating-point suport.

Anyone see a reason why it would not meed its stated goals, or
otherwise see a defect or possible improvement, including comments?


// Conversion from UTC date to second, unsigned 32-bit Unix epoch version.
// Written by François Grieu, 2015-07-21; public domain.

#include <time.h>                   // needed for struct tm
#include <limits.h>                 // needed for UINT_MAX
#if UINT_MAX>=0xFFFFFFFF            // unsigned is at least 32-bit
typedef unsigned      my_time_t;    // type for seconds since 1970
#else
typedef unsigned long my_time_t;    // type for seconds since 1970
#endif

// my_mktime  converts from  struct tm  UTC to non-leap seconds since
// 00:00:00 on the first UTC day of year 1970 (fixed).
// It works from 1970 to 2105 inclusive. It strives to be compatible
// with C compilers supporting // comments and claiming C89 conformance.
//
// input:   Pointer to a  struct tm  with field tm_year, tm_mon, tm_mday,
//          tm_hour, tm_min, tm_sec set per  mktime  convention; thus
//          - tm_year is year minus 1900
//          - tm_mon is [0..11] for January to December, but [-2..14]
//            works for November of previous year to February of next year
//          - tm_mday, tm_hour, tm_min, tm_sec similarly can be offset to
//            the full range [-32767 to 32768], as long as the combination
//            with tm_year gives a result within years [1970..2105], and
//            tm_year>0.
// output:  Number of non-leap seconds since beginning of the first UTC
//          day of year 1970, as an unsigned at-least-32-bit integer.
//          The input is not changed (in particular, fields tm_wday,
//          tm_yday, and tm_isdst are unchanged and ignored).
my_time_t my_mktime(const struct tm * ptm) {
     int m, y = ptm->tm_year;
     if ((m = ptm->tm_mon)<2) { m += 12; --y; }
     return ((( (my_time_t)(y-69)*365u+y/4-y/100*3/4+(m+2)*153/5-446+
         ptm->tm_mday)*24u+ptm->tm_hour)*60u+ptm->tm_min)*60u+ptm->tm_sec;
     }


TIA (and excuse the hickup)

    François Grieu

Note: code also posted to http://stackoverflow.com/a/31533119/903600
together with a more general version featuring adjustable epoch and
much larger range, aiming at C99 conformance.

[toc] | [next] | [standalone]

#66285

Francois Grieu <fgrieu@gmail.com> writes:

> The following aims at converting from UTC date in  struct tm  format
> to seconds, with portability in mind, including to systems with
> 16-bit int or/and lacking 64-bit and floating-point suport.
>
> Anyone see a reason why it would not meed its stated goals, or
> otherwise see a defect or possible improvement, including comments?
>
>
> // Conversion from UTC date to second, unsigned 32-bit Unix epoch version.
> // Written by Francois Grieu, 2015-07-21; public domain.
>
> #include <time.h>                   // needed for struct tm
> #include <limits.h>                 // needed for UINT_MAX
> #if UINT_MAX>=0xFFFFFFFF            // unsigned is at least 32-bit
> typedef unsigned      my_time_t;    // type for seconds since 1970
> #else
> typedef unsigned long my_time_t;    // type for seconds since 1970
> #endif
>
> // my_mktime  converts from  struct tm  UTC to non-leap seconds since
> // 00:00:00 on the first UTC day of year 1970 (fixed).
> // It works from 1970 to 2105 inclusive.  It strives to be compatible
> // with C compilers supporting // comments and claiming C89 conformance.
> //
> // input:   Pointer to a  struct tm  with field tm_year, tm_mon, tm_mday,
> //          tm_hour, tm_min, tm_sec set per  mktime  convention;  thus
> //          - tm_year is year minus 1900
> //          - tm_mon is [0..11] for January to December, but [-2..14]
> //            works for November of previous year to February of next year
> //          - tm_mday, tm_hour, tm_min, tm_sec similarly can be offset to
> //            the full range [-32767 to 32768], as long as the combination
> //            with tm_year gives a result within years [1970..2105], and
> //            tm_year>0.
> // output:  Number of non-leap seconds since beginning of the first UTC
> //          day of year 1970, as an unsigned at-least-32-bit integer.
> //          The input is not changed (in particular, fields tm_wday,
> //          tm_yday, and tm_isdst are unchanged and ignored).
> my_time_t my_mktime(const struct tm * ptm) {
>     int m, y = ptm->tm_year;
>     if ((m = ptm->tm_mon)<2) { m += 12; --y; }
>     return ((( (my_time_t)(y-69)*365u+y/4-y/100*3/4+(m+2)*153/5-446+
>         ptm->tm_mday)*24u+ptm->tm_hour)*60u+ptm->tm_min)*60u+ptm->tm_sec;
>     }

Some remarks and reactions...

Given that you are aiming for C89/C90 conformance, I don't see
why you don't use /*..*/ comments exclusively.  Maybe you have a
reason that you're not saying, but in the absence of such I think
it would be better just to limit the commenting to C90 style.

My understanding is type names ending in _t, like my_time_t, are
usually reserved for use by POSIX (and sometimes C, I guess).  So
it seems advisable to choose a different name for the return
type.  And, as far as that goes, why not just use unsigned long?
Also, the way the pre-processor is used to build the typedef for
your time type is a little too tricky IMO.  It seems better to
just write the two alternatives out in full, and use #if to
select between them.

I don't understand why you talk about UTC in the description.
ISTM that the code you have will work for any "time zone", as
long as the input argument and the output number are understood
to be in the same time zone, and the time zone does not have an
adjustment for Daylight Stupid Time.

The description of the input format is fairly good, except you
might want to note that tm_mday starts at 1, not 0.

However, despite the last comment, it isn't clear what range of
inputs are expected to work, or whether strange values in the
various fields (eg, tm_month of -30000) will work.  If the fields
are signed and the return type is unsigned, there are likely to
be problems with signed to unsigned conversion in the arithmetic.
So, two conclusions:  one, I recommend writing a more specific
description of what inputs are acceptable;  and two, if any input
field is allowed to have a negative value, there should be some
commentary explaining why the code given should produce the right
result.  (I have not tested any inputs with negative field
values.)

The code in the function body is pretty ugly.  No, let's make
that very ugly.  An example suggested improvement:

    int m = ptm->tm_mon  + (ptm->tm_mon < 2 ? 12 : 0);
    int y = ptm->tm_year - (ptm->tm_mon < 2);

I also recommend the return statement be re-written to be more
easily digestible, as for example

    unsigned long days = ...;
    unsigned long seconds = ...;
    return  days * 86400 + seconds;

I expect reading these three lines will be faster (and easier)
than reading the two lines of the existing return statement.  The
return statement that is there now just falls below threshold for
me.

Besides matters of compositional style, the algorithm you're
using is, well, let's say not very transparent.  It took me a
long time to see what you're doing there.  That isn't necessarily
a bad thing;  personally I think I would prefer a more direct
expression of the different parts that contribute, but maybe
combining the different parts the way you have done is okay.
However, at the very least it deserves an explanatory comment,
probably at least a paragraph - otherwise the bafflement level
is just outside a reasonable range.  (btw I'm impressed by the
algorithm, it's quite clever;  but that doesn't change my other
remarks above.)

FWIW the code seems to produce the right values in the
tests that I ran.

> Note: code also posted to http://stackoverflow.com/a/31533119/903600
> together with a more general version featuring adjustable epoch and
> much larger range, aiming at C99 conformance.

If you post it here I will try to take a look at it.

[toc] | [prev] | [next] | [standalone]

#66292

Tim Rentsch <txr@alumni.caltech.edu> writes:
[...]
> My understanding is type names ending in _t, like my_time_t, are
> usually reserved for use by POSIX

That's my understanding as well, though I haven't been able to find the
specific requirement.  (Searching the POSIX web site for "reserved" is
difficult when every page has "All Rights Reserved" at the bottom;
searching for "_t" isn't much better.)

>                                   (and sometimes C, I guess).

ISO C does not reserve identifiers ending with _t in general.  Some
specific type names (size_t, time_t) are of course reserved.

[...]

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#66293

On 07/24/2015 01:36 PM, Keith Thompson wrote:
> Tim Rentsch <txr@alumni.caltech.edu> writes:
> [...]
>> My understanding is type names ending in _t, like my_time_t, are
>> usually reserved for use by POSIX
> 
> That's my understanding as well, though I haven't been able to find the
> specific requirement.  (Searching the POSIX web site for "reserved" is
> difficult when every page has "All Rights Reserved" at the bottom;
> searching for "_t" isn't much better.)

Search for "reserved identifier". When I did so, the first hit was
section 2.2.2, containing a complete list of reservations, with a suffix
of "_t" being reserved for "Any header".

The second hit was B.2.12, which provides an explanation for that rule.
I couldn't find confirmation of my understanding that identifiers ending
in _t are reserved specifically for use as type names.

[toc] | [prev] | [next] | [standalone]

#66304

James Kuyper <jameskuyper@verizon.net> writes:
> On 07/24/2015 01:36 PM, Keith Thompson wrote:
>> Tim Rentsch <txr@alumni.caltech.edu> writes:
>> [...]
>>> My understanding is type names ending in _t, like my_time_t, are
>>> usually reserved for use by POSIX
>> 
>> That's my understanding as well, though I haven't been able to find the
>> specific requirement.  (Searching the POSIX web site for "reserved" is
>> difficult when every page has "All Rights Reserved" at the bottom;
>> searching for "_t" isn't much better.)
>
> Search for "reserved identifier". When I did so, the first hit was
> section 2.2.2, containing a complete list of reservations, with a suffix
> of "_t" being reserved for "Any header".

Got it.  It's the last entry in the second table in 2.2.2.  (I think the
table is split into two parts just because of its size.)

http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html

> The second hit was B.2.12, which provides an explanation for that rule.
> I couldn't find confirmation of my understanding that identifiers ending
> in _t are reserved specifically for use as type names.

It wouldn't make much sense to reserve them only for use as type names,
since type names aren't in their own namespace.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#66308

On 07/24/2015 03:21 PM, Keith Thompson wrote:
> James Kuyper <jameskuyper@verizon.net> writes:
...
>> The second hit was B.2.12, which provides an explanation for that rule.
>> I couldn't find confirmation of my understanding that identifiers ending
>> in _t are reserved specifically for use as type names.
> 
> It wouldn't make much sense to reserve them only for use as type names,
> since type names aren't in their own namespace.

Sorry - that was bad wording on my part. I don't mean that the
reservation applies only to type names. I mean that when POSIX uses
identifiers of that type, it uses them exclusively as type names. I've
heard it claimed that there is such a rule, and every example I can
think of fits that rule, but I haven't located the actual text of that rule.

[toc] | [prev] | [next] | [standalone]

#66414

Keith Thompson <kst-u@mib.org> writes:

> Tim Rentsch <txr@alumni.caltech.edu> writes:
> [...]
>> My understanding is type names ending in _t, like my_time_t, are
>> usually reserved for use by POSIX
>
> That's my understanding as well, though I haven't been able to find the
> specific requirement.  (Searching the POSIX web site for "reserved" is
> difficult when every page has "All Rights Reserved" at the bottom;
> searching for "_t" isn't much better.)
>
>>                                   (and sometimes C, I guess).
>
> ISO C does not reserve identifiers ending with _t in general.  Some
> specific type names (size_t, time_t) are of course reserved.
>
> [...]

Right, that's what I meant.  Also it appears that the Standard
is allowed to "claim" new identifiers ending in _t for itself
even though they are not listed as reserved identifiers (eg, in
C11 there is a new type 'errno_t').  That second aspect is also
what I was referring to in my earlier comment.

[toc] | [prev] | [next] | [standalone]

#66415

Keith Thompson <kst-u@mib.org> writes:

> Tim Rentsch <txr@alumni.caltech.edu> writes:
> [...]
>> My understanding is type names ending in _t, like my_time_t, are
>> usually reserved for use by POSIX
>
> That's my understanding as well, though I haven't been able to find the
> specific requirement.  (Searching the POSIX web site for "reserved" is
> difficult when every page has "All Rights Reserved" at the bottom;
> searching for "_t" isn't much better.)
>
>>                                   (and sometimes C, I guess).
>
> ISO C does not reserve identifiers ending with _t in general.  Some
> specific type names (size_t, time_t) are of course reserved.
>
> [...]

The document from POSIX 2004 at

http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_02.html

mentions: "Implementations may add symbols to the headers shown in the
following table provided the identifiers for those symbols either:

- Begin with the corresponding reserved prefixes in the table, or

- Have one of the corresponding complete names in the table, or

- End in the string indicated as a reserved suffix in the table and do
  not use the reserved prefixes posix_, POSIX_, or _POSIX_, as long as
  the reserved suffix is in that part of the name considered significant
  by the implementation."

The table ends with a line listing _t as a reserved suffix in any header.

POSIX has a huge list of reserved names... The GNU libc manual has an
overview of reserved names:

http://www.gnu.org/software/libc/manual/html_mono/libc.html#Reserved-Names

but when comparing with the POSIX docs this list seems outdated.

-- Alain.

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web