[systemd-devel] [PATCH 1/2] timedatectl: check for getenv("TZDIR")

[Kay Sievers]

On Tue, Mar 24, 2015 at 7:39 PM, Shawn Landden <shawn at churchofgit.com> wrote:
> On Tue, Mar 24, 2015 at 11:32 AM, Kay Sievers <kay at vrfy.org> wrote:
>> On Tue, Mar 24, 2015 at 7:11 PM, Shawn Landden <shawn at churchofgit.com> wrote:
>>
>>>          /* Enforce the values of /etc/localtime */
>>>          if (getenv("TZ")) {
>>> -                fprintf(stderr, "Warning: Ignoring the TZ variable.\n\n");
>>> +                fprintf(stderr, "Warning: Ignoring the %s variable.\n\n", "TZ");
>>
>> What style is that? A second static string inserted with %s?
>> Wanto de-duplicate the first static string? :)
> yep

Come on, we are not starting to smart-out the compiler and trade size
against runtime at the same time. Not that it matters, but it makes no
sense for systemd or that tool. Please do not do that.

>>
>>> -                xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime_r(&sec, &tm));
>>> +                xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime);
>>
>> Why this change?
> localtime_r() and gmtime_r() are called multiple times.

And what's the problem? If this is again to optimize things, please
don't do that in systemd code unless it makes things better readable
at the same time, the *_r functions are generally preferred.

>>> +                      "         RTC in UTC by calling 'timedatectl set-local-rtc 0'\n
>>> +                      "         For more details see http://www.cl.cam.ac.uk/~mgk25/mswish/ut-rtc.html" ANSI_HIGHLIGHT_OFF ".\n", stdout);
>>
>> Hmm, I find links to "random" web pages in error output a bit too
>> unconventional.
> OK, but the page has been around for years and does tell Windows users
> what registry entry to set to use UTC for their RTC.

Hmm, this is nothing to suggest to Windows users. Windows will stop to
touch the RTC and never adjust it again. This would only work for
Linux boxes where Windows is only booted for a short while and then
booted back to Linux. We can't know that.

Kay