* Re: [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
2016-02-16 10:29 ` Karel Zak
@ 2016-02-16 13:54 ` Sami Kerola
2016-02-16 20:13 ` Ruediger Meier
2016-02-17 2:02 ` J William Piggott
2 siblings, 0 replies; 9+ messages in thread
From: Sami Kerola @ 2016-02-16 13:54 UTC (permalink / raw)
To: Karel Zak; +Cc: J William Piggott, util-linux, Ruediger Meier
On 16 February 2016 at 10:29, Karel Zak <kzak@redhat.com> wrote:
> On Thu, Feb 11, 2016 at 07:48:40PM -0500, J William Piggott wrote:
>> - lt = localtime(&hwctime.tv_sec);
>> - strftime(ctime_now, sizeof(ctime_now), format, lt);
>> - printf(_("%s and %06d microseconds\n"), ctime_now, (int)hwctime.tv_usec);
>> + struct tm lt;
>> + int zhour, zmin;
>> +
>> + lt = *localtime(&hwctime.tv_sec);
>> + zhour = - timezone / 60 / 60;
>> + zmin = abs(timezone / 60 % 60);
>> + printf(_("%4d-%.2d-%.2d %02d:%02d:%02d.%06d%+02d:%02d\n"),
>> + lt.tm_year + 1900, lt.tm_mon + 1, lt.tm_mday, lt.tm_hour,
>> + lt.tm_min, lt.tm_sec, (int)hwctime.tv_usec, zhour, zmin);
>
> Some notes:
>
> * what's wrong with strftime?
>
> * We already use ISO time in util-linux and we use time designator 'T' (separator
> between date and time).
>
> * tv_usec is "long"
>
> * fraction separator is decimal mark, either a comma or a dot, but
> with a preference for a comma according to ISO 8601:2004
> (wikipedia).
>
> -- we already use comma in util-linux, exception is logger where I
> see dot :-(
>
>
> Anyway, it would be really nice to have a function for this purpose in
> lib/timeutils.c to avoid duplication and creativity :-) Something like:
>
>
> strtime_iso_8601(char buf, size_t bufsz, struct tm tm, struct timeval frac, int flags);
>
> where flags are
>
> enum {
> ISO_8601_TIMEZONE = (1 << 1)
> ISO_8601_USEC = (1 << 2)
> ...
> };
>
> then we can use this function in lslogins, dmesg, hwclock, last,
> logger, ... etc. I can also imagine:
>
> strtime_short_ctime()
> strtime_ctime() (ctime without \n)
>
> Comments?
Logger is special, it is using Internet date/time format, see 5.8. Examples.
https://www.ietf.org/rfc/rfc3339.txt
And when I read the ISO 8601 from wikipedia
https://en.wikipedia.org/wiki/ISO_8601#Times
there is: A decimal mark, either a comma or a dot (without any
preference as stated in resolution 10 of the 22nd General Conference
CGPM in 2003, but with a preference for a comma according to ISO
8601:2004)
So one hopes date parser authors will accept either.
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
2016-02-16 10:29 ` Karel Zak
2016-02-16 13:54 ` Sami Kerola
@ 2016-02-16 20:13 ` Ruediger Meier
2016-02-17 2:02 ` J William Piggott
2 siblings, 0 replies; 9+ messages in thread
From: Ruediger Meier @ 2016-02-16 20:13 UTC (permalink / raw)
To: Karel Zak; +Cc: J William Piggott, util-linux, Sami Kerola
On Tuesday 16 February 2016, Karel Zak wrote:
> On Thu, Feb 11, 2016 at 07:48:40PM -0500, J William Piggott wrote:
> > - lt = localtime(&hwctime.tv_sec);
> > - strftime(ctime_now, sizeof(ctime_now), format, lt);
> > - printf(_("%s and %06d microseconds\n"), ctime_now,
> > (int)hwctime.tv_usec); + struct tm lt;
> > + int zhour, zmin;
> > +
> > + lt = *localtime(&hwctime.tv_sec);
> > + zhour = - timezone / 60 / 60;
> > + zmin = abs(timezone / 60 % 60);
> > + printf(_("%4d-%.2d-%.2d %02d:%02d:%02d.%06d%+02d:%02d\n"),
> > + lt.tm_year + 1900, lt.tm_mon + 1, lt.tm_mday, lt.tm_hour,
> > + lt.tm_min, lt.tm_sec, (int)hwctime.tv_usec, zhour, zmin);
>
> Some notes:
>
> * what's wrong with strftime?
For me strftime always "sounds" locale and system dependent. If we don't
want to use locale anyways then I would prefer printf to make this
clear for the reader.
On the other hand if someone knows that our choosen ISO style is
portable and locale independent implemented in any system's strftime,
ok to use strftime then.
> * We already use ISO time in util-linux and we use time designator
> 'T' (separator between date and time).
I know that "T" is the best compromise. Anyways for humans I try to use
space if possible. It simply looks better. IMO more important is that
any parser code would accept space as well as 'T'.
> * tv_usec is "long"
>
> * fraction separator is decimal mark, either a comma or a dot, but
> with a preference for a comma according to ISO 8601:2004
> (wikipedia).
>
> -- we already use comma in util-linux, exception is logger where I
> see dot :-(
Where do we use comma? Usually I would prefer dot (for decimal fractions
of seconds). I guess most people use dot and would be confused about
comma. Existing software may more likely not accept comma. Like this
example
https://github.com/moment/moment/issues/2580
> Anyway, it would be really nice to have a function for this purpose
> in lib/timeutils.c to avoid duplication and creativity :-) Something
> like:
>
>
> strtime_iso_8601(char buf, size_t bufsz, struct tm tm, struct
> timeval frac, int flags);
>
> where flags are
>
> enum {
> ISO_8601_TIMEZONE = (1 << 1)
> ISO_8601_USEC = (1 << 2)
> ...
> };
>
> then we can use this function in lslogins, dmesg, hwclock, last,
> logger, ... etc. I can also imagine:
>
> strtime_short_ctime()
> strtime_ctime() (ctime without \n)
>
> Comments?
>
> Karel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
2016-02-16 10:29 ` Karel Zak
2016-02-16 13:54 ` Sami Kerola
2016-02-16 20:13 ` Ruediger Meier
@ 2016-02-17 2:02 ` J William Piggott
2016-02-17 2:20 ` Ruediger Meier
2016-02-17 10:54 ` Karel Zak
2 siblings, 2 replies; 9+ messages in thread
From: J William Piggott @ 2016-02-17 2:02 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux, Ruediger Meier, Sami Kerola
On 02/16/2016 05:29 AM, Karel Zak wrote:
> On Thu, Feb 11, 2016 at 07:48:40PM -0500, J William Piggott wrote:
>> - lt = localtime(&hwctime.tv_sec);
>> - strftime(ctime_now, sizeof(ctime_now), format, lt);
>> - printf(_("%s and %06d microseconds\n"), ctime_now, (int)hwctime.tv_usec);
>> + struct tm lt;
>> + int zhour, zmin;
>> +
>> + lt = *localtime(&hwctime.tv_sec);
>> + zhour = - timezone / 60 / 60;
>> + zmin = abs(timezone / 60 % 60);
>> + printf(_("%4d-%.2d-%.2d %02d:%02d:%02d.%06d%+02d:%02d\n"),
>> + lt.tm_year + 1900, lt.tm_mon + 1, lt.tm_mday, lt.tm_hour,
>> + lt.tm_min, lt.tm_sec, (int)hwctime.tv_usec, zhour, zmin);
>
> Some notes:
>
> * what's wrong with strftime?
Why use resources on strftime when 8601 prints directly?
While it can do other formatting, I see strftime's main purpose as
converting tm numbers into strings like: January, Monday, etc.
>
> * We already use ISO time in util-linux and we use time designator 'T' (separator
> between date and time).
I chose to use the optional space as a compromise between machine
friendly and human friendly, this is commonly done I think.
>
> * tv_usec is "long"
Fixed: I followed the example from the recent commit c211401 (for code
consistency, which is why I originally used the int cast), and cast it
to long; although I do not understand why as it is already long. All
that needed to be done was to change the conversion length modifier and
drop the cast.
PULL:
git@github.com:jwpi/util-linux.git 020716
for you to fetch changes up to e05ac5aae00913e7a999c96a7bb731dc1d09cc5c:
>
> * fraction separator is decimal mark, either a comma or a dot, but
> with a preference for a comma according to ISO 8601:2004
> (wikipedia).
>
> -- we already use comma in util-linux, exception is logger where I
> see dot :-(
I actually modeled the format after:
date --rfc-3339=ns
2016-02-16 20:13:25.601508440-05:00
3339 is a subset of 8601 that requires using a period for the radix
point. As I said previously, I chose this as a compromise between
machine and human readability. As it is output to tty by default, I
thought doing so might mitigate any push back against the format change.
I am not strongly opposed to using 'T' as the date-time delimiter or comma
as the radix point. I just think 'space' and 'period' are a commonly
used compromise.
>
>
> Anyway, it would be really nice to have a function for this purpose in
> lib/timeutils.c to avoid duplication and creativity :-) Something like:
>
>
> strtime_iso_8601(char buf, size_t bufsz, struct tm tm, struct timeval frac, int flags);
>
> where flags are
>
> enum {
> ISO_8601_TIMEZONE = (1 << 1)
> ISO_8601_USEC = (1 << 2)
> ...
> };
>
> then we can use this function in lslogins, dmesg, hwclock, last,
> logger, ... etc. I can also imagine:
>
> strtime_short_ctime()
> strtime_ctime() (ctime without \n)
>
> Comments?
Also, perhaps all the above only using something similar to coreutil's
fprintftime, so it prints directly using less resources.
>
> Karel
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
2016-02-17 2:02 ` J William Piggott
@ 2016-02-17 2:20 ` Ruediger Meier
2016-02-17 10:54 ` Karel Zak
1 sibling, 0 replies; 9+ messages in thread
From: Ruediger Meier @ 2016-02-17 2:20 UTC (permalink / raw)
To: J William Piggott; +Cc: Karel Zak, util-linux, Sami Kerola
On Wednesday 17 February 2016, J William Piggott wrote:
> On 02/16/2016 05:29 AM, Karel Zak wrote:
> > On Thu, Feb 11, 2016 at 07:48:40PM -0500, J William Piggott wrote:
> >> - lt = localtime(&hwctime.tv_sec);
> >> - strftime(ctime_now, sizeof(ctime_now), format, lt);
> >> - printf(_("%s and %06d microseconds\n"), ctime_now,
> >> (int)hwctime.tv_usec); + struct tm lt;
> >> + int zhour, zmin;
> >> +
> >> + lt = *localtime(&hwctime.tv_sec);
> >> + zhour = - timezone / 60 / 60;
> >> + zmin = abs(timezone / 60 % 60);
> >> + printf(_("%4d-%.2d-%.2d %02d:%02d:%02d.%06d%+02d:%02d\n"),
> >> + lt.tm_year + 1900, lt.tm_mon + 1, lt.tm_mday,
> >> lt.tm_hour, + lt.tm_min, lt.tm_sec, (int)hwctime.tv_usec,
> >> zhour, zmin);
> >
> > Some notes:
> >
> > * what's wrong with strftime?
>
> Why use resources on strftime when 8601 prints directly?
>
> While it can do other formatting, I see strftime's main purpose as
> converting tm numbers into strings like: January, Monday, etc.
>
> > * We already use ISO time in util-linux and we use time designator
> > 'T' (separator between date and time).
>
> I chose to use the optional space as a compromise between machine
> friendly and human friendly, this is commonly done I think.
>
> > * tv_usec is "long"
>
> Fixed: I followed the example from the recent commit c211401 (for
> code consistency, which is why I originally used the int cast), and
> cast it to long; although I do not understand why as it is already
> long. All that needed to be done was to change the conversion length
> modifier and drop the cast.
It's not long on all systems, e.g.__darwin_suseconds_t is int. Casting
to long seems to be the way to avoid warnings everywhere.
> PULL:
>
> git@github.com:jwpi/util-linux.git 020716
>
> for you to fetch changes up to
e05ac5aae00913e7a999c96a7bb731dc1d09cc5c:
> > * fraction separator is decimal mark, either a comma or a dot, but
> > with a preference for a comma according to ISO 8601:2004
> > (wikipedia).
> >
> > -- we already use comma in util-linux, exception is logger where
> > I see dot :-(
>
> I actually modeled the format after:
>
> date --rfc-3339=ns
> 2016-02-16 20:13:25.601508440-05:00
>
> 3339 is a subset of 8601 that requires using a period for the radix
> point. As I said previously, I chose this as a compromise between
> machine and human readability. As it is output to tty by default, I
> thought doing so might mitigate any push back against the format
> change.
>
> I am not strongly opposed to using 'T' as the date-time delimiter or
> comma as the radix point. I just think 'space' and 'period' are a
> commonly used compromise.
>
> > Anyway, it would be really nice to have a function for this purpose
> > in lib/timeutils.c to avoid duplication and creativity :-)
> > Something like:
> >
> >
> > strtime_iso_8601(char buf, size_t bufsz, struct tm tm, struct
> > timeval frac, int flags);
> >
> > where flags are
> >
> > enum {
> > ISO_8601_TIMEZONE = (1 << 1)
> > ISO_8601_USEC = (1 << 2)
> > ...
> > };
> >
> > then we can use this function in lslogins, dmesg, hwclock, last,
> > logger, ... etc. I can also imagine:
> >
> > strtime_short_ctime()
> > strtime_ctime() (ctime without \n)
> >
> > Comments?
>
> Also, perhaps all the above only using something similar to
> coreutil's fprintftime, so it prints directly using less resources.
>
> > Karel
>
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux"
> in the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
2016-02-17 2:02 ` J William Piggott
2016-02-17 2:20 ` Ruediger Meier
@ 2016-02-17 10:54 ` Karel Zak
2016-02-17 11:05 ` Karel Zak
1 sibling, 1 reply; 9+ messages in thread
From: Karel Zak @ 2016-02-17 10:54 UTC (permalink / raw)
To: J William Piggott; +Cc: util-linux, Ruediger Meier, Sami Kerola
On Tue, Feb 16, 2016 at 09:02:39PM -0500, J William Piggott wrote:
>
> On 02/16/2016 05:29 AM, Karel Zak wrote:
> > On Thu, Feb 11, 2016 at 07:48:40PM -0500, J William Piggott wrote:
> >> - lt = localtime(&hwctime.tv_sec);
> >> - strftime(ctime_now, sizeof(ctime_now), format, lt);
> >> - printf(_("%s and %06d microseconds\n"), ctime_now, (int)hwctime.tv_usec);
> >> + struct tm lt;
> >> + int zhour, zmin;
> >> +
> >> + lt = *localtime(&hwctime.tv_sec);
> >> + zhour = - timezone / 60 / 60;
> >> + zmin = abs(timezone / 60 % 60);
> >> + printf(_("%4d-%.2d-%.2d %02d:%02d:%02d.%06d%+02d:%02d\n"),
> >> + lt.tm_year + 1900, lt.tm_mon + 1, lt.tm_mday, lt.tm_hour,
> >> + lt.tm_min, lt.tm_sec, (int)hwctime.tv_usec, zhour, zmin);
> >
> > Some notes:
> >
> > * what's wrong with strftime?
>
> Why use resources on strftime when 8601 prints directly?
>
> While it can do other formatting, I see strftime's main purpose as
> converting tm numbers into strings like: January, Monday, etc.
It does more things for you, see lt.tm_year + 1900, lt.tm_mon + 1,
calculation of the time zone hours, timezone minutes...
> > Anyway, it would be really nice to have a function for this purpose in
> > lib/timeutils.c to avoid duplication and creativity :-) Something like:
> >
> >
> > strtime_iso_8601(char buf, size_t bufsz, struct tm tm, struct timeval frac, int flags);
> >
> > where flags are
> >
> > enum {
> > ISO_8601_TIMEZONE = (1 << 1)
> > ISO_8601_USEC = (1 << 2)
> > ...
> > };
> >
> > then we can use this function in lslogins, dmesg, hwclock, last,
> > logger, ... etc. I can also imagine:
> >
> > strtime_short_ctime()
> > strtime_ctime() (ctime without \n)
> >
> > Comments?
>
> Also, perhaps all the above only using something similar to coreutil's
> fprintftime, so it prints directly using less resources.
Well, things don't end after commit. Someone has to maintain it for
years and for example now we have many places where we produces the
same time format. So, I'd like to keep it on one place and minimize
differences in formatting.
I'll merge your patch and cleanup the things later...
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
2016-02-17 10:54 ` Karel Zak
@ 2016-02-17 11:05 ` Karel Zak
0 siblings, 0 replies; 9+ messages in thread
From: Karel Zak @ 2016-02-17 11:05 UTC (permalink / raw)
To: J William Piggott; +Cc: util-linux, Ruediger Meier, Sami Kerola
On Wed, Feb 17, 2016 at 11:54:32AM +0100, Karel Zak wrote:
> On Tue, Feb 16, 2016 at 09:02:39PM -0500, J William Piggott wrote:
> >
> > On 02/16/2016 05:29 AM, Karel Zak wrote:
> > > On Thu, Feb 11, 2016 at 07:48:40PM -0500, J William Piggott wrote:
> > >> - lt = localtime(&hwctime.tv_sec);
> > >> - strftime(ctime_now, sizeof(ctime_now), format, lt);
> > >> - printf(_("%s and %06d microseconds\n"), ctime_now, (int)hwctime.tv_usec);
> > >> + struct tm lt;
> > >> + int zhour, zmin;
> > >> +
> > >> + lt = *localtime(&hwctime.tv_sec);
> > >> + zhour = - timezone / 60 / 60;
> > >> + zmin = abs(timezone / 60 % 60);
> > >> + printf(_("%4d-%.2d-%.2d %02d:%02d:%02d.%06d%+02d:%02d\n"),
> > >> + lt.tm_year + 1900, lt.tm_mon + 1, lt.tm_mday, lt.tm_hour,
> > >> + lt.tm_min, lt.tm_sec, (int)hwctime.tv_usec, zhour, zmin);
> > >
> > > Some notes:
> > >
> > > * what's wrong with strftime?
> >
> > Why use resources on strftime when 8601 prints directly?
> >
> > While it can do other formatting, I see strftime's main purpose as
> > converting tm numbers into strings like: January, Monday, etc.
>
> It does more things for you, see lt.tm_year + 1900, lt.tm_mon + 1,
> calculation of the time zone hours, timezone minutes...
Ah, now I see that strftime is not able to produce timezone hh:mm.
Well, compose the string without strftime seems better in this case.
Sorry.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 9+ messages in thread