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] |
On 06/07/2013 06:23 AM, Corinna Vinschen wrote:
setenv() already has the tzset() call. Adding it to unsetenv(), also, to fix the latent bug will make no size difference to anyone's build other than the patch itself, as you would not be using unsetenv() without having used setenv().Hi Craig, On Jun 6 17:33, Craig Howland wrote:On 07/09/2011 08:19 AM, Corinna Vinschen wrote:Having said that, shouldn't the tzset() call better be removed in _setenv_r and called in the affected functions?Corinna: It's been a while, but to return to this since I'm running into it again and would like to get away from a local workaround: Good question. But, no, I suggest that it is better to make the addition to unsetenv() rather than to use your suggestion for footprint reasons (in addition to my original less-likely-to-cause-backwards-compatibility reason). If we were to remove tzset() from setenv and add it to ctime(), localtime(), mktime(), and strftime(), we would be forcing the environment functions to be linked when any of those 4 functions were called. [...] Having tzset() called by unsetenv() and setenv() can equally be claimed to be pulling tzset() in when perhaps it might not be wanted, but this seems a more likely case than the former. That is, if you are using environment variables, it seems much more likely that you'd also be using time functions at the same time than the flip case.I disagree. The fact that some target uses environment functions is no conclusive reason to believe that the application uses time functions *and* needs TZ.[...] In summary, I think that technically either calling tzset() from the related time functions or from the environment functions could work properly and be standard-compliant. But I think that the environment method better fits newlib's desire for small footprints, and suggest that route.Let's have a look into the footprints of the object files. I took the two targets I have handy all the time. The size is the combined size of text, data, rdata and bss segments, in bytes (ctime_r.o and lcltime_r.o are irrelevant; they just call other functions): i686 x86_64 setenv_r.o 844 880 getenv_r.o 288 256 mktime.o 2064 2224 mktm_r.o 1808 2000 strftime.o 8004 8416 tzset_r.o 1388 1600 Tzset alone is bigger than setenv/getenv combined, so by adding a tzset call you raise the size of an executable using the environment functions by more than 100%.
OK. While I disagree that this is the best approach, it is your prerogative and attached is a patch for the ctime()/localtime()/mktime()/strftime() approach. It does have the benefit of also fixing the problem identified by Yaakov in "[PATCH] strftime: use tzname if TM_ZONE is NULL" (https://sourceware.org/ml/newlib/2015/msg00321.html). Since ctime() calls localtime(), it is not directly patched (i.e. the patch only directly adds to 3 of the 4 functions mentioned). The strftime.c patch also gets the wcsftime() function due to the shared source. I made a minor size/efficiency call in strftime(), and put in two tzset() calls, one for %z and one for %Z, so that tzset() will only be called if it is needed, at the expense of one more function call in the overall size. (Which is what Corinna says GLIBC does.0... So, to me it looks much more reasonable to call tzset from the time functions. ... Corinna
Craig
Attachment:
tzset.ChangeLog.txt
Description: Text document
Attachment:
tzset.patch.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |