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

[Corinna Vinschen <vinschen at redhat dot com>]

On Sep  4 12:56, Freddie Chopin wrote:
> 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?

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?

Yes, certainly.  In this scenario, since _gmtime_r will be included
as soon as localtime_r is used, the arrays would just go into the
_gmtime_r source file as well.

> >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).

Not these, but comments never hurt :)


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgpkTd4Jb7jRV.pgp
Description: PGP signature