This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Shortcomings in time calculations (time zones, timegm, leap years)


Hi all:

First of all, many thanks for Newlib.

I am writing firmware for a family of small embedded devices that live in different time zones and have to do extensive time calculations, including leap years. This is painful with Newlib.

I am grateful that you have implemented 64-bit time_t recently (see USE_LONG_TIME_T). I can now stop using the following "y2038" library, which wasn't a smooth ride either:

  https://github.com/evalEmpire/y2038

I had to jump through many hoops to get some error information out of Newlib's tzset(), and avoid leaks on invalid input. That is actually the only place in my entire firmware that needs environment variables at all. The gory details are here:

  Difficulties with tzset() and setenv()
  https://sourceware.org/ml/newlib/2017/msg00700.html

The one thing I miss dearly is timegm(), which does the inverse of gmtime(), and is not standard, but widely available. More information about this is here:

https://stackoverflow.com/questions/38298261/why-there-is-no-inverse-function-for-gmtime-in-libc

The Boost implementation is small and could serve as a basis, look for internal_timegm() here:

https://github.com/boostorg/chrono/blob/develop/include/boost/chrono/io/time_point_io.hpp

But I suspect that such a routine could share source code with the existing mktime() implementation. For example, both routines share the same code in this project:

https://github.com/freebsd/freebsd/blob/master/contrib/tzcode/stdtime/localtime.c

In the process, I would make these macros in mktime.c public (or even better, as inline functions), as it would be very convenient to have such code readily available and well tested:

#define _ISLEAP(y) (((y) % 4) == 0 && (((y) % 100) != 0 || (((y)+1900) % 400) == 0))
  ---> but beware of the "+1900" inside _ISLEAP!
I would add a comment like "returns exactly 0 or 1, as it can be directly used as an array index"

  #define _DAYS_IN_YEAR(year) (_ISLEAP(year) ? 366 : 365)

I know it's not much, but it saves searching around for something suitable, which actually took me longer than I expected.

In fact, the logic is duplicated in Newlib already, see also is_leap_year() in strptime.c and isleap() in local.h (which does not seem to be a public header).

Array __month_lengths[] inside local.h would also be a good candidate to be made public. Furthermore, 72 bytes of program memory could be saved if it were an array of uint8_t instead of 32-bit integers.

Furthermore, it would save memory and performance if there were alternative entry points for routines like mktime() that just took the time zone as a parameter. This way, there would be no need to do a TZ_LOCK, and ultimately no need to have an environment at all for embedded targets.

I cannot directly contribute source code at the moment. But I hope that this mailing list message may at least help others identify these shortcomings.

Regards,
  rdiez


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]