5

I am trying to implement in C two simple convertors, date/time to time-stamp and vice-versa, without any dependencies on time library routines (such as localtime, mktime, etc, mainly due to the fact that some of them are thread-unsafe).

I have previously posted a similar question under Convert date/time to time-stamp and vice versa, and would now like to submit it again with a few notable changes:

I have the following date/time structure:

typedef struct
{
    unsigned char second; // 0-59
    unsigned char minute; // 0-59
    unsigned char hour;   // 0-59
    unsigned char day;    // 1-31
    unsigned char month;  // 1-12
    unsigned char year;   // 0-99 (representing 2000-2099)
}
date_time_t;

I would like to have a second opinion on the following conversion routines (given a legal input):

static unsigned short days[4][12] =
{
    {   0,  31,  60,  91, 121, 152, 182, 213, 244, 274, 305, 335},
    { 366, 397, 425, 456, 486, 517, 547, 578, 609, 639, 670, 700},
    { 731, 762, 790, 821, 851, 882, 912, 943, 974,1004,1035,1065},
    {1096,1127,1155,1186,1216,1247,1277,1308,1339,1369,1400,1430},
};


unsigned int date_time_to_epoch(date_time_t* date_time)
{
    unsigned int second = date_time->second;  // 0-59
    unsigned int minute = date_time->minute;  // 0-59
    unsigned int hour   = date_time->hour;    // 0-23
    unsigned int day    = date_time->day-1;   // 0-30
    unsigned int month  = date_time->month-1; // 0-11
    unsigned int year   = date_time->year;    // 0-99
    return (((year/4*(365*4+1)+days[year%4][month]+day)*24+hour)*60+minute)*60+second;
}


void epoch_to_date_time(date_time_t* date_time,unsigned int epoch)
{
    date_time->second = epoch%60; epoch /= 60;
    date_time->minute = epoch%60; epoch /= 60;
    date_time->hour   = epoch%24; epoch /= 24;

    unsigned int years = epoch/(365*4+1)*4; epoch %= 365*4+1;

    unsigned int year;
    for (year=3; year>0; year--)
    {
        if (epoch >= days[year][0])
            break;
    }

    unsigned int month;
    for (month=11; month>0; month--)
    {
        if (epoch >= days[year][month])
            break;
    }

    date_time->year  = years+year;
    date_time->month = month+1;
    date_time->day   = epoch-days[year][month]+1;
}

I have tested this on an extensive amount of legal input (between 01/01/2000 and 31/12/2099). Any constructive comments would be appreciated (performance improvement suggestions, readability, etc)...

UPDATE - my ultimate goal here (due to which I am posting this question):

I have an STM32 (ARM-based cortex), with a timer configured to interrupt the CPU every 10ms. In addition, I have an RTC connected, from which I can read date/time (in 1sec resolution). Accessing the RTC is less efficient, so I would like to read it only once, and from then on, calculate the date/time using the 10ms timer interrupts. I wish to avoid using 'localtime', since I then have to protect it with a mutex. The only solution that comes to mind is implementing my own 'localtime', and as subsequent result - my own 'mktime' as well (my epoch in the code above counts seconds from the beginning of the year 2000).

Community
  • 1
  • 1
barak manos
  • 29,648
  • 10
  • 62
  • 114
  • I am in a hurry right now so can't read and analyse your code but just a suggestion. You might also like to post this in [StackExchange Code Review](http://codereview.stackexchange.com/) – erosenin Dec 29 '13 at 15:44
  • 5
    This question appears to be off-topic because it is about doing a code review. If you know your code is buggy, specify what inputs break it and explain what you've tried to fix it. If you think your code is ok and want a review, post on http://codereview.stackexchange.com. If you're not sure whether your code is buggy or not, test it. – Mat Dec 29 '13 at 15:45
  • @erosenin, thanks for the advice (was not aware of this forum/website) – barak manos Dec 29 '13 at 15:52
  • 2
    This approach looks much too complicated to me, if it is only for the goal of thread safety. `mktime` and `difftime` *are* thread safe. It is possible to implement wrappers for the remaining time functions from there. You can view such an implementation of the C11 functions from Annex K (with names ending in `_s`) that are thread safe in P99: p99.gforge.inria.fr – Jens Gustedt Dec 29 '13 at 16:05
  • @JensGustedt +!, plus: a lot of implementations already have had non-standard reentrant versions of time functions, like localtime_r, for a long time because of this problem. If you are not working on building a cross-platform app consider that as well. – jim mcnamara Dec 29 '13 at 16:18
  • @Mat - good point (although I admit that I initially took it as a non-constructive comment). Thanks. – barak manos Dec 29 '13 at 17:51
  • 1
    If you want your implementation to be _value-compatible_ with time_t/unix timestamps, you'll need to handle leap-seconds, too, plus: support for adding them _in the future_. – wildplasser Dec 29 '13 at 17:58
  • @wildplasser AFAIK, unix timestamps don't include leap seconds. – Matt Johnson-Pint Dec 29 '13 at 21:24
  • @Matt Johnson The C11 7.27.1 contains the footnote about `struct tm`: "The range [0, 60] for `tm_sec` allows for a positive leap second". – chux - Reinstate Monica Dec 29 '13 at 21:32
  • @wildplasser - Yes, `struct tm` does allow for a leap second. But a unix timestamp is simply the number of seconds since 1/1/1970 UTC - ignoring leap seconds. – Matt Johnson-Pint Dec 29 '13 at 21:39
  • 1
    @Mat,wildplasser: The date/time I need to convert comes from an RTC on an STM32 (ARM based cortex). So all I care about is to be able to convert this date/time "back and forth" (don't care about unix or any other OS for that matter). Are you suggesting that I must check whether or not this RTC includes leap seconds? – barak manos Dec 29 '13 at 21:41
  • @MattJohnson: posix says it is implementation defined how timestamp corresponds to UTC. [posix timestamp itself knows nothing about leap seconds](http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_15) – jfs Dec 29 '13 at 21:41
  • 1
    @J.F.Sebastian - Good reference, and much better wording. Yes, it's implementation defined, and most implementations (but not all) ignore it. – Matt Johnson-Pint Dec 29 '13 at 21:43
  • @barakmanos - It depends on how accurate you need to be. If you are doing something where the timing is sensitive, then yes - you will need to consult a table of leap seconds after reading from your RTC and decide how to adjust. – Matt Johnson-Pint Dec 29 '13 at 21:46
  • 1
    You may have problems with the year 2100 if you are relying on a 4-year cycle; see http://en.wikipedia.org/wiki/Century_leap_year –  Dec 29 '13 at 22:22
  • @wilsonmichaelpatrick: See "unsigned char year; // 0-99 (representing 2000-2099)" at the beginning of the post. Thanks – barak manos Dec 29 '13 at 22:26
  • Note: STM32 may use 32-bit `unsigned int epoch`, but `unsigned int` is only guaranteed to range _at least_ 0 to 65535 which is insufficient for a century of seconds. For compatibility, suggest using `unsigned long` – chux - Reinstate Monica Dec 29 '13 at 22:33
  • This question is now also on http://codereview.stackexchange.com/questions/38275/convert-between-date-time-and-time-stamp-without-using-std-library-routines/38302#38302 – chux - Reinstate Monica Dec 29 '13 at 23:40
  • epoch uses 32bit signed. Yes, "signed". – Mohammad Kholghi May 29 '21 at 08:47

1 Answers1

1

For a performance improvement, consider to not doing epoch_to_date_time() every second (or even every timer tick), but to selectively increment a time unit only when the lesser unit overflows, e. g. like

void another_second_passed(date_time_t *date_time)
{   // *date_time to persist from call to call, initialized once from RTC
    if (++date_time->second < 60) return;   // finished in 59 of 60 cases
    date_time->second = 0;
    if (++date_time->minute < 60) return;   // finished in 59 of 60 cases
    date_time->minute = 0;
    …
}
Armali
  • 18,255
  • 14
  • 57
  • 171