Util-Linux package development
 help / color / mirror / Atom feed
* [PATCH 0/1] hwclock --show format
@ 2016-02-12  0:42 J William Piggott
  2016-02-12  0:48 ` [PATCH 1/1] hwclock.c, hwclock.8.in: new " J William Piggott
  0 siblings, 1 reply; 9+ messages in thread
From: J William Piggott @ 2016-02-12  0:42 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux


The following commit is incorrect.
 _______________________
commit dadaad018721a82aafc06af2e94c1c2f8cef68de
Author: Anna Jonna Ã<81>rmannsdóttir <annajonna@gmail.com>
Date:   Thu Jan 28 19:03:20 2016 +0000

    Change of output format and documentation.
 _______________________

RE: hwclock.8.in changes

hwclock --show does not use the date(1) format, it uses strftime(3) %c.

Under POSIX date(1) format has a specific definition. In the glibc
locale database it is set to the keyword date_fmt; hwclock uses
strftime(3) %c format which is keyword d_t_fmt. For example:

LC_TIME=en_HK locale -k date_fmt d_t_fmt
date_fmt="%a %b %e %H:%M:%S %Z %Y"
d_t_fmt="%A, %B %d, %Y %p%I:%M:%S %Z"

LC_TIME=en_US locale -k date_fmt d_t_fmt
date_fmt="%a %b %e %H:%M:%S %Z %Y"
d_t_fmt="%a %d %b %Y %r %Z"

For the above locales date(1) format is the same, but hwclock(8) (strftime
%c) is very different:

LC_TIME=en_HK date; LC_TIME=en_US date
Thu Feb 11 16:58:23 EST 2016
Thu Feb 11 16:58:23 EST 2016

LC_TIME=en_HK /sbin/hwclock; LC_TIME=en_US /sbin/hwclock
Thursday, February 11, 2016 PM04:59:34 EST  .559820 seconds
Thu 11 Feb 2016 04:59:35 PM EST  .004092 seconds


RE: hwclock.c changes:

OLD: Thu 11 Feb 2016 03:41:17 PM EST  .490796 seconds
NEW: Thu 11 Feb 2016 03:40:55 PM EST and 000850 microseconds

"hwclock --show" is supposed to display the Hardware Clock time; it is
not a dialog. Adding more and longer words, IMO, is not a improvement.
(There also should not have been any zero padding.)

The apparent purpose of this change is to address the ambiguity created
by having the fractional seconds separate from the time display. In the
past it had to be that way, because the fractional part was always
negative and did not wrap on overflow. After fixing that, I intended to
concatenate the fractional part to the time display; using the %c format
made doing so non-trivial. I wanted to change to a different format, but
I had assumed that there would be push back against it. So the task went
to the bottom of my TODO list.

Now that a format change has already been committed, I will go with that and
submit a further change to ISO 8601.

RATIONAL

As hwclock is a system administration tool having its --show function in
the ISO 8601 format is appropriate because:

* the format should be machine friendly
* the format should be conducive to logging
* the format should be conducive for timestamping
* it should use a predictable, locale independent, international format
* it does not need to pretty print because:
    * it cannot be called by a normal user
    * it is not used for wallclock time


PULL REQUEST INFORMATION

The following changes since commit 583627ef363f0e0bd8a7dac646507c2458accc56:

  agetty: add support for \e in issue file to print \033 (2016-02-11 12:33:57 +0100)

are available in the git repository at:

  git@github.com:jwpi/util-linux.git 020716

for you to fetch changes up to 0c0d33d1f6d564fdc16a655edecda38db74bce0e:

  hwclock.c, hwclock.8.in: new --show format (2016-02-11 16:02:05 -0500)

----------------------------------------------------------------
J William Piggott (1):
      hwclock.c, hwclock.8.in: new --show format

 sys-utils/hwclock.8.in |  8 +++-----
 sys-utils/hwclock.c    | 16 +++++++++-------
 2 files changed, 12 insertions(+), 12 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
  2016-02-12  0:42 [PATCH 0/1] hwclock --show format J William Piggott
@ 2016-02-12  0:48 ` J William Piggott
  2016-02-16 10:29   ` Karel Zak
  0 siblings, 1 reply; 9+ messages in thread
From: J William Piggott @ 2016-02-12  0:48 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux


* hwclock.c: change --get and --show functions to the ISO 8601
  format and concatenate fractional seconds to the time display.

* hwclock.8.in: document this.

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 sys-utils/hwclock.8.in |  8 +++-----
 sys-utils/hwclock.c    | 16 +++++++++-------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/sys-utils/hwclock.8.in b/sys-utils/hwclock.8.in
index d72b7a8..eddb864 100644
--- a/sys-utils/hwclock.8.in
+++ b/sys-utils/hwclock.8.in
@@ -88,16 +88,14 @@ command, such as \%'11\ minute\ mode' or from dual-booting another OS.
 .TQ
 .B \-\-get
 .br
-Read the Hardware Clock and print the time on standard output.
+Read the Hardware Clock and print its time to standard output in the
+.B ISO 8601
+format.
 The time shown is always in local time, even if you keep your Hardware Clock
 in UTC.  See the
 .B \%\-\-localtime
 option.
 .sp
-The time shown is in same format as that of 
-.BR \%date (1)
-by default, and additionally, the number of microseconds is shown.
-.sp 
 Showing the Hardware Clock time is the default when no function is specified.
 .sp
 The
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 2eb21ac..6f14081 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -687,13 +687,15 @@ display_time(const bool hclock_valid, struct timeval hwctime)
 		       "either invalid (e.g. 50th day of month) or beyond the range "
 		       "we can handle (e.g. Year 2095)."));
 	else {
-		struct tm *lt;
-		char *format = "%c";
-		char ctime_now[200];
-
-		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);
 	}
 }
 

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/1] hwclock.c, hwclock.8.in: new --show format
  2016-02-12  0:48 ` [PATCH 1/1] hwclock.c, hwclock.8.in: new " J William Piggott
@ 2016-02-16 10:29   ` Karel Zak
  2016-02-16 13:54     ` Sami Kerola
                       ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Karel Zak @ 2016-02-16 10:29 UTC (permalink / raw)
  To: J William Piggott; +Cc: util-linux, Ruediger Meier, Sami Kerola

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?

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

end of thread, other threads:[~2016-02-17 11:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-12  0:42 [PATCH 0/1] hwclock --show format J William Piggott
2016-02-12  0:48 ` [PATCH 1/1] hwclock.c, hwclock.8.in: new " J William Piggott
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
2016-02-17 11:05         ` Karel Zak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox