Re: [PATCH] libc/time/gmtime_r.c, libc/time/lcltime_r.c,, libc/time/local.h, libc/time/mktm_r.c: move localtime related functionality, from _mktm_r() to new _mklocaltm_r() to break dependency of gmtime() on, timezones

[Freddie Chopin <freddie_chopin at op dot pl>]

W dniu 2014-09-04 10:36, Corinna Vinschen pisze:
> How does that work?  _mktm_r and _mklocaltm_r are still in the same source
> file, so if you get one, you also get the other.  Where did you get your
> savings from?

Hmm... My tests were done by compiling newlib sources as part of the 
project, and I always use "-ffunction-sections -fdata-sections" for 
compilation and "--gc-sections" for linking, so that unused functions 
are removed even if they are in the same file.

I guess that for this savings to work in "normal" scenario (newlib used 
as a library) these would really need to be split, right?

Initially I planned to split these functions to separate files, but I 
didn't feel like modifying makefiles and there's the problem of sharing 
the arrays (all of these functions use the array with number of days in 
the months).

If such separation is required, I'd like to extend the change further, 
doing deeper changes that would actually simplify stuff. First of all - 
in my opinion - this _mktm_r() is actually not very useful (in the 
patched version) - it's just gmtime_r() - I'd like to move all this code 
to gmtime_r(). This new function which does time zone adjustments - 
_mklocaltm_r() - is not really useful alone too - I'd like to move all 
that code to localtime_r(), which would initially just call gmtime_r(), 
then do the time zone adjustments on the result. The only local/internal 
function that would be left would be __tzcalc_limits() which is used by 
3 other functions. Array mon_lengths[] from mktm_r.c would probably need 
to be put in separate source file - it is used by _mktm_r() 
(gmtime_r()), _mklocaltm_r() (localtime_r()) and __tzcalc_limits().

Does that sound OK? Would such change be acceptable?

> Since these are internal functions, I think the names are fine.  Comments
> preceding _mktm_r and _mklocaltm_r would be nice.

Right, I'll add proper comments in the next version if these would be 
required (if the plan above would be acceptable, these local functions 
would be gone, so no comments needed).

Regards,
FCh