* [PATCH 0/6] Pull Request
@ 2017-08-19 0:16 J William Piggott
2017-08-19 0:18 ` [PATCH 1/6] hwclock: move systz into hctosys J William Piggott
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: J William Piggott @ 2017-08-19 0:16 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
The following changes since commit 6047c6db9c261c8048c54279ba1206ad0ca5d61e:
lslogins: be more explicit with -g in man page (2017-08-18 10:29:54 +0200)
are available in the git repository at:
git@github.com:jwpi/util-linux.git 170803
for you to fetch changes up to 6524846ce501f7cfb9cc9a5ba369f37ef47d5ea2:
hwclock: for debugging print startup system time (2017-08-18 16:07:28 -0400)
----------------------------------------------------------------
J William Piggott (6):
hwclock: move systz into hctosys
hwclock: remove set_system_clock_timezone()
hwclock: update set_system_clock debugging
hwclock: update set_system_clock comments
hwclock: rename/refactor set_system_clock()
hwclock: for debugging print startup system time
sys-utils/hwclock.c | 187 ++++++++++++++++++----------------------------------
1 file changed, 63 insertions(+), 124 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/6] hwclock: move systz into hctosys
2017-08-19 0:16 [PATCH 0/6] Pull Request J William Piggott
@ 2017-08-19 0:18 ` J William Piggott
2017-08-19 0:19 ` [PATCH 2/6] hwclock: remove set_system_clock_timezone() J William Piggott
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: J William Piggott @ 2017-08-19 0:18 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
The set_system_clock_timezone() function is nearly identical to
set_system_clock(). Three additional statements are required
to include systz in hctosys.
This patch is intentionally incomplete to make reviewing the
actual required changes easier. Other patches in this set will:
* remove set_system_clock_timezone()
* fix messages and debugging
* fix comments
* and finally refactor set_system_clock()
Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
sys-utils/hwclock.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 910e39f..e49f1a1 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -594,6 +594,7 @@ set_system_clock(const struct hwclock_control *ctl,
struct tm *broken;
int minuteswest;
int rc = 0;
+ const struct timezone tz_utc = { 0 };
broken = localtime(&newtime.tv_sec);
#ifdef HAVE_TM_GMTOFF
@@ -621,10 +622,15 @@ set_system_clock(const struct hwclock_control *ctl,
* mode does not clobber the Hardware Clock with UTC. This
* is only available on first call of settimeofday after boot.
*/
- if (!ctl->universal)
+ if (ctl->hctosys && !ctl->universal) /* set PCIL */
rc = settimeofday(tv_null, &tz);
- if (!rc)
+ if (ctl->systz && ctl->universal) /* lock warp_clock */
+ rc = settimeofday(tv_null, &tz_utc);
+ if (!rc && ctl->hctosys)
rc = settimeofday(&newtime, &tz);
+ else if (!rc)
+ rc = settimeofday(NULL, &tz);
+
if (rc) {
warn(_("settimeofday() failed"));
retcode = 1;
@@ -1024,7 +1030,7 @@ manipulate_clock(const struct hwclock_control *ctl, const time_t set_time,
}
if (ctl->systz)
- return set_system_clock_timezone(ctl);
+ return set_system_clock(ctl, startup_time);
if (ur->get_permissions())
return EX_NOPERM;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/6] hwclock: remove set_system_clock_timezone()
2017-08-19 0:16 [PATCH 0/6] Pull Request J William Piggott
2017-08-19 0:18 ` [PATCH 1/6] hwclock: move systz into hctosys J William Piggott
@ 2017-08-19 0:19 ` J William Piggott
2017-08-19 0:20 ` [PATCH 3/6] hwclock: update set_system_clock debugging J William Piggott
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: J William Piggott @ 2017-08-19 0:19 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
Remove set_system_clock_timezone() because the previous patch
moved its functionality into set_system_clock().
Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
sys-utils/hwclock.c | 89 -----------------------------------------------------
1 file changed, 89 deletions(-)
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index e49f1a1..ddd6413 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -641,95 +641,6 @@ set_system_clock(const struct hwclock_control *ctl,
}
/*
- * Reset the System Clock from local time to UTC, based on its current value
- * and the timezone unless universal is TRUE.
- *
- * Also set the kernel time zone value to the value indicated by the TZ
- * environment variable and/or /usr/lib/zoneinfo/, interpreted as tzset()
- * would interpret them.
- *
- * If 'testing' is true, don't actually update anything -- just say we would
- * have.
- */
-static int set_system_clock_timezone(const struct hwclock_control *ctl)
-{
- int retcode;
- struct timeval tv;
- struct tm *broken;
- int minuteswest;
-
- gettimeofday(&tv, NULL);
- if (ctl->debug) {
- struct tm broken_time;
- char ctime_now[200];
-
- broken_time = *gmtime(&tv.tv_sec);
- strftime(ctime_now, sizeof(ctime_now), "%Y/%m/%d %H:%M:%S",
- &broken_time);
- printf(_("Current system time: %ld = %s\n"), tv.tv_sec,
- ctime_now);
- }
-
- broken = localtime(&tv.tv_sec);
-#ifdef HAVE_TM_GMTOFF
- minuteswest = -broken->tm_gmtoff / 60; /* GNU extension */
-#else
- minuteswest = timezone / 60;
- if (broken->tm_isdst)
- minuteswest -= 60;
-#endif
-
- if (ctl->debug) {
- struct tm broken_time;
- char ctime_now[200];
-
- gettimeofday(&tv, NULL);
- if (!ctl->universal)
- tv.tv_sec += minuteswest * 60;
-
- broken_time = *gmtime(&tv.tv_sec);
- strftime(ctime_now, sizeof(ctime_now), "%Y/%m/%d %H:%M:%S",
- &broken_time);
-
- printf(_("Calling settimeofday:\n"));
- printf(_("\tUTC: %s\n"), ctime_now);
- printf(_("\ttv.tv_sec = %ld, tv.tv_usec = %ld\n"),
- tv.tv_sec, tv.tv_usec);
- printf(_("\ttz.tz_minuteswest = %d\n"), minuteswest);
- }
- if (ctl->testing) {
- printf(_
- ("Test mode: clock was not changed\n"));
- retcode = 0;
- } else {
- const struct timezone tz_utc = { 0, 0 };
- const struct timezone tz = { minuteswest, 0 };
- const struct timeval *tv_null = NULL;
- int rc = 0;
-
- /* The first call to settimeofday after boot will assume the systemtime
- * is in localtime, and adjust it according to the given timezone to
- * compensate. If the systemtime is in fact in UTC, then this is wrong
- * so we first do a dummy call to make sure the time is not shifted.
- */
- if (ctl->universal)
- rc = settimeofday(tv_null, &tz_utc);
-
- /* Now we set the real timezone. Due to the above dummy call, this will
- * only warp the systemtime if the RTC is not in UTC. */
- if (!rc)
- rc = settimeofday(tv_null, &tz);
-
- if (rc) {
- warn(_("settimeofday() failed"));
- retcode = 1;
- } else
- retcode = 0;
- }
- return retcode;
-}
-
-/*
* Refresh the last calibrated and last adjusted timestamps in <*adjtime_p>
* to facilitate future drift calculations based on this set point.
*
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/6] hwclock: update set_system_clock debugging
2017-08-19 0:16 [PATCH 0/6] Pull Request J William Piggott
2017-08-19 0:18 ` [PATCH 1/6] hwclock: move systz into hctosys J William Piggott
2017-08-19 0:19 ` [PATCH 2/6] hwclock: remove set_system_clock_timezone() J William Piggott
@ 2017-08-19 0:20 ` J William Piggott
2017-08-19 0:22 ` [PATCH 4/6] hwclock: update set_system_clock comments J William Piggott
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: J William Piggott @ 2017-08-19 0:20 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
Update debug messages for a combined --systz and --hctosys in
the set_system_clock function.
New debug messages:
hwclock --test -D --systz --localtime
Calling settimeofday(NULL, 240) to warp System time.
Test mode: clock was not changed
hwclock --test -D --systz --utc
Calling settimeofday(NULL, 0) to lock the warp function.
Calling settimeofday(NULL, 240) to set the kernel timezone.
Test mode: clock was not changed
hwclock --test -D --hctosys --utc
Calling settimeofday(1502239269.733639, 240)
Test mode: clock was not changed
hwclock --test -D --hctosys --localtime
Calling settimeofday(NULL, 240) to set persistent_clock_is_local.
Calling settimeofday(1502253708.200200, 240)
Test mode: clock was not changed
Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
sys-utils/hwclock.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index ddd6413..0be9c2a 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -606,11 +606,24 @@ set_system_clock(const struct hwclock_control *ctl,
#endif
if (ctl->debug) {
- printf(_("Calling settimeofday:\n"));
- printf(_("\ttv.tv_sec = %ld, tv.tv_usec = %ld\n"),
- newtime.tv_sec, newtime.tv_usec);
- printf(_("\ttz.tz_minuteswest = %d\n"), minuteswest);
+ if (ctl->hctosys && !ctl->universal)
+ printf(_("Calling settimeofday(NULL, %d) to set "
+ "persistent_clock_is_local.\n"), minuteswest);
+ if (ctl->systz && ctl->universal)
+ puts(_("Calling settimeofday(NULL, 0) "
+ "to lock the warp function."));
+ if (ctl->hctosys)
+ printf(_("Calling settimeofday(%ld.%06ld, %d)\n"),
+ newtime.tv_sec, newtime.tv_usec, minuteswest);
+ else {
+ printf(_("Calling settimeofday(NULL, %d) "), minuteswest);
+ if (ctl->universal)
+ puts(_("to set the kernel timezone."));
+ else
+ puts(_("to warp System time."));
+ }
}
+
if (ctl->testing) {
printf(_
("Test mode: clock was not changed\n"));
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/6] hwclock: update set_system_clock comments
2017-08-19 0:16 [PATCH 0/6] Pull Request J William Piggott
` (2 preceding siblings ...)
2017-08-19 0:20 ` [PATCH 3/6] hwclock: update set_system_clock debugging J William Piggott
@ 2017-08-19 0:22 ` J William Piggott
2017-08-19 0:23 ` [PATCH 5/6] hwclock: rename/refactor set_system_clock() J William Piggott
2017-08-19 0:24 ` [PATCH 6/6] hwclock: for debugging print startup system time J William Piggott
5 siblings, 0 replies; 12+ messages in thread
From: J William Piggott @ 2017-08-19 0:22 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
sys-utils/hwclock.c | 42 +++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 0be9c2a..c398d60 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -569,20 +569,36 @@ display_time(struct timeval hwctime)
}
/*
- * Set the System Clock to time 'newtime'.
+ * Adjusts System time, sets the kernel's timezone and RTC timescale.
*
- * Also set the kernel time zone value to the value indicated by the TZ
- * environment variable and/or /usr/lib/zoneinfo/, interpreted as tzset()
- * would interpret them.
+ * The kernel warp_clock function adjusts the System time according to the
+ * tz.tz_minuteswest argument and sets PCIL (see below). At boot settimeofday(2)
+ * has one-shot access to this function as shown in the table below.
*
- * If this is the first call of settimeofday since boot, then this also sets
- * the kernel variable persistent_clock_is_local so that NTP 11 minute mode
- * will update the Hardware Clock with the proper timescale. If the Hardware
- * Clock's timescale configuration is changed then a reboot is required for
- * persistent_clock_is_local to be updated.
+ * +-------------------------------------------------------------------+
+ * | settimeofday(tv, tz) |
+ * |-------------------------------------------------------------------|
+ * | Arguments | System Time | PCIL | | warp_clock |
+ * | tv | tz | set | warped | set | firsttime | locked |
+ * |---------|---------|---------------|------|-----------|------------|
+ * | pointer | NULL | yes | no | no | 1 | no |
+ * | pointer | pointer | yes | no | no | 0 | yes |
+ * | NULL | ptr2utc | no | no | no | 0 | yes |
+ * | NULL | pointer | no | yes | yes | 0 | yes |
+ * +-------------------------------------------------------------------+
+ * ptr2utc: tz.tz_minuteswest is zero (UTC).
+ * PCIL: persistent_clock_is_local, sets the "11 minute mode" timescale.
+ * firsttime: locks the warp_clock function (initialized to 1 at boot).
*
- * If 'testing' is true, don't actually update anything -- just say we would
- * have.
+ * +---------------------------------------------------------------------------+
+ * | op | RTC scale | settimeofday calls |
+ * |---------|-----------|-----------------------------------------------------|
+ * | systz | Local | 1) warps system time*, sets PCIL* and kernel tz |
+ * | systz | UTC | 1st) locks warp_clock* 2nd) sets kernel tz |
+ * | hctosys | Local | 1st) sets PCIL* 2nd) sets system time and kernel tz |
+ * | hctosys | UTC | 1) sets system time and kernel tz |
+ * +---------------------------------------------------------------------------+
+ * * only on first call after boot
*/
static int
set_system_clock(const struct hwclock_control *ctl,
@@ -631,10 +647,6 @@ set_system_clock(const struct hwclock_control *ctl,
} else {
const struct timezone tz = { minuteswest, 0 };
- /* Set kernel persistent_clock_is_local so that 11 minute
- * mode does not clobber the Hardware Clock with UTC. This
- * is only available on first call of settimeofday after boot.
- */
if (ctl->hctosys && !ctl->universal) /* set PCIL */
rc = settimeofday(tv_null, &tz);
if (ctl->systz && ctl->universal) /* lock warp_clock */
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/6] hwclock: rename/refactor set_system_clock()
2017-08-19 0:16 [PATCH 0/6] Pull Request J William Piggott
` (3 preceding siblings ...)
2017-08-19 0:22 ` [PATCH 4/6] hwclock: update set_system_clock comments J William Piggott
@ 2017-08-19 0:23 ` J William Piggott
2017-08-21 9:31 ` Karel Zak
2017-08-19 0:24 ` [PATCH 6/6] hwclock: for debugging print startup system time J William Piggott
5 siblings, 1 reply; 12+ messages in thread
From: J William Piggott @ 2017-08-19 0:23 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
sys-utils/hwclock.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index c398d60..4279931 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -601,12 +601,8 @@ display_time(struct timeval hwctime)
* * only on first call after boot
*/
static int
-set_system_clock(const struct hwclock_control *ctl,
- const struct timeval newtime)
+kernel_time_ctl(const struct hwclock_control *ctl, const struct timeval newtime)
{
- int retcode;
-
- const struct timeval *tv_null = NULL;
struct tm *broken;
int minuteswest;
int rc = 0;
@@ -643,14 +639,13 @@ set_system_clock(const struct hwclock_control *ctl,
if (ctl->testing) {
printf(_
("Test mode: clock was not changed\n"));
- retcode = 0;
} else {
- const struct timezone tz = { minuteswest, 0 };
+ const struct timezone tz = { minuteswest };
if (ctl->hctosys && !ctl->universal) /* set PCIL */
- rc = settimeofday(tv_null, &tz);
+ rc = settimeofday(NULL, &tz);
if (ctl->systz && ctl->universal) /* lock warp_clock */
- rc = settimeofday(tv_null, &tz_utc);
+ rc = settimeofday(NULL, &tz_utc);
if (!rc && ctl->hctosys)
rc = settimeofday(&newtime, &tz);
else if (!rc)
@@ -658,11 +653,10 @@ set_system_clock(const struct hwclock_control *ctl,
if (rc) {
warn(_("settimeofday() failed"));
- retcode = 1;
- } else
- retcode = 0;
+ return 1;
+ }
}
- return retcode;
+ return 0;
}
/*
@@ -966,7 +960,7 @@ manipulate_clock(const struct hwclock_control *ctl, const time_t set_time,
}
if (ctl->systz)
- return set_system_clock(ctl, startup_time);
+ return kernel_time_ctl(ctl, startup_time);
if (ur->get_permissions())
return EX_NOPERM;
@@ -1036,7 +1030,7 @@ manipulate_clock(const struct hwclock_control *ctl, const time_t set_time,
adjust_drift_factor(ctl, adjtime, nowtime,
hclocktime);
} else if (ctl->hctosys) {
- return set_system_clock(ctl, hclocktime);
+ return kernel_time_ctl(ctl, hclocktime);
}
if (!ctl->noadjfile)
save_adjtime(ctl, adjtime);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/6] hwclock: for debugging print startup system time
2017-08-19 0:16 [PATCH 0/6] Pull Request J William Piggott
` (4 preceding siblings ...)
2017-08-19 0:23 ` [PATCH 5/6] hwclock: rename/refactor set_system_clock() J William Piggott
@ 2017-08-19 0:24 ` J William Piggott
5 siblings, 0 replies; 12+ messages in thread
From: J William Piggott @ 2017-08-19 0:24 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
sys-utils/hwclock.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 4279931..72f1f09 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -1364,8 +1364,11 @@ int main(int argc, char **argv)
}
#endif
- if (ctl.debug)
+ if (ctl.debug) {
out_version();
+ printf(_("System Time: %ld.%06ld\n"),
+ startup_time.tv_sec, startup_time.tv_usec);
+ }
if (!ctl.systz && !ctl.predict)
determine_clock_access_method(&ctl);
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] hwclock: rename/refactor set_system_clock()
2017-08-19 0:23 ` [PATCH 5/6] hwclock: rename/refactor set_system_clock() J William Piggott
@ 2017-08-21 9:31 ` Karel Zak
2017-08-21 12:56 ` J William Piggott
0 siblings, 1 reply; 12+ messages in thread
From: Karel Zak @ 2017-08-21 9:31 UTC (permalink / raw)
To: J William Piggott; +Cc: util-linux
On Fri, Aug 18, 2017 at 08:23:01PM -0400, J William Piggott wrote:
>
> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> ---
> sys-utils/hwclock.c | 24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
> index c398d60..4279931 100644
> --- a/sys-utils/hwclock.c
> +++ b/sys-utils/hwclock.c
> @@ -601,12 +601,8 @@ display_time(struct timeval hwctime)
> * * only on first call after boot
> */
> static int
> -set_system_clock(const struct hwclock_control *ctl,
> - const struct timeval newtime)
> +kernel_time_ctl(const struct hwclock_control *ctl, const struct timeval newtime)
Hmm... the new name does not make the code more readable for me.
We have "HW-clock" (hc) and "system time", would be nice to keep
this terminology for functions names too?
IMHO set_system_clock() is not so bad :-) (Maybe use system_set_clock()
if you want to use "system_" prefix for more functions.)
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] hwclock: rename/refactor set_system_clock()
2017-08-21 9:31 ` Karel Zak
@ 2017-08-21 12:56 ` J William Piggott
2017-08-22 9:04 ` Karel Zak
0 siblings, 1 reply; 12+ messages in thread
From: J William Piggott @ 2017-08-21 12:56 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On 08/21/2017 05:31 AM, Karel Zak wrote:
> On Fri, Aug 18, 2017 at 08:23:01PM -0400, J William Piggott wrote:
>>
>> Signed-off-by: J William Piggott <elseifthen@gmx.com>
>> ---
>> sys-utils/hwclock.c | 24 +++++++++---------------
>> 1 file changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
>> index c398d60..4279931 100644
>> --- a/sys-utils/hwclock.c
>> +++ b/sys-utils/hwclock.c
>> @@ -601,12 +601,8 @@ display_time(struct timeval hwctime)
>> * * only on first call after boot
>> */
>> static int
>> -set_system_clock(const struct hwclock_control *ctl,
>> - const struct timeval newtime)
>> +kernel_time_ctl(const struct hwclock_control *ctl, const struct timeval newtime)
>
> Hmm... the new name does not make the code more readable for me.
>
> We have "HW-clock" (hc) and "system time", would be nice to keep
> this terminology for functions names too?
I've been moving away from the term 'Hardware Clock' in favor of RTC
because:
* use of /dev/rtcN is the only recommended access now
* RTC seems more common (anecdotal observation)
* it is more concise for usage() and docs
Some people do not like the term "Real Time Clock", but I have not seen a
consensus for any name.
>
> IMHO set_system_clock() is not so bad :-) (Maybe use system_set_clock()
> if you want to use "system_" prefix for more functions.)
My thinking was that this function does important things besides setting
the system time. It also sets the kernel's RTC timescale and the
kernel's timezone. For systz it never sets the system time at all. So
the current name seems misleading.
It doesn't matter to me, I was changing it to help others. Do you want me
to put it back to set_system_clock?
>
> Karel
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] hwclock: rename/refactor set_system_clock()
2017-08-21 12:56 ` J William Piggott
@ 2017-08-22 9:04 ` Karel Zak
2017-08-24 23:35 ` J William Piggott
0 siblings, 1 reply; 12+ messages in thread
From: Karel Zak @ 2017-08-22 9:04 UTC (permalink / raw)
To: J William Piggott; +Cc: util-linux
On Mon, Aug 21, 2017 at 08:56:41AM -0400, J William Piggott wrote:
>
>
> On 08/21/2017 05:31 AM, Karel Zak wrote:
> > On Fri, Aug 18, 2017 at 08:23:01PM -0400, J William Piggott wrote:
> >>
> >> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> >> ---
> >> sys-utils/hwclock.c | 24 +++++++++---------------
> >> 1 file changed, 9 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
> >> index c398d60..4279931 100644
> >> --- a/sys-utils/hwclock.c
> >> +++ b/sys-utils/hwclock.c
> >> @@ -601,12 +601,8 @@ display_time(struct timeval hwctime)
> >> * * only on first call after boot
> >> */
> >> static int
> >> -set_system_clock(const struct hwclock_control *ctl,
> >> - const struct timeval newtime)
> >> +kernel_time_ctl(const struct hwclock_control *ctl, const struct timeval newtime)
> >
> > Hmm... the new name does not make the code more readable for me.
> >
> > We have "HW-clock" (hc) and "system time", would be nice to keep
> > this terminology for functions names too?
>
> I've been moving away from the term 'Hardware Clock' in favor of RTC
> because:
> * use of /dev/rtcN is the only recommended access now
> * RTC seems more common (anecdotal observation)
> * it is more concise for usage() and docs
RTC is fine.
> > IMHO set_system_clock() is not so bad :-) (Maybe use system_set_clock()
> > if you want to use "system_" prefix for more functions.)
>
> My thinking was that this function does important things besides setting
> the system time. It also sets the kernel's RTC timescale and the
> kernel's timezone. For systz it never sets the system time at all. So
> the current name seems misleading.
kernel_time_ctl() does not inform reader about any operation, as there
is no "set", "get", "read", "manipulate" or so in the name.
> It doesn't matter to me, I was changing it to help others. Do you want me
> to put it back to set_system_clock?
Please, or try something better (set_time(), set_kernel_time(), ...).
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] hwclock: rename/refactor set_system_clock()
2017-08-22 9:04 ` Karel Zak
@ 2017-08-24 23:35 ` J William Piggott
2017-08-25 6:48 ` Karel Zak
0 siblings, 1 reply; 12+ messages in thread
From: J William Piggott @ 2017-08-24 23:35 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On 08/22/2017 05:04 AM, Karel Zak wrote:
> On Mon, Aug 21, 2017 at 08:56:41AM -0400, J William Piggott wrote:
>>
8< ...
>
>> It doesn't matter to me, I was changing it to help others. Do you want me
>> to put it back to set_system_clock?
>
> Please, or try something better (set_time(), set_kernel_time(), ...).
Kept the original name set_system_clock and pushed to the same branch:
The following changes since commit ebfab541cf1266bf80b2b62977cf29ff69c61fe1:
fdisk: add missing include (2017-08-24 19:59:20 +0200)
are available in the git repository at:
git@github.com:jwpi/util-linux.git 170803
for you to fetch changes up to e406be01b450dd7ed5e694bc15a39719d6f86967:
hwclock: for debugging print startup system time (2017-08-24 18:49:11 -0400)
----------------------------------------------------------------
J William Piggott (6):
hwclock: move systz into hctosys
hwclock: remove set_system_clock_timezone()
hwclock: update set_system_clock debugging
hwclock: update set_system_clock comments
hwclock: refactor set_system_clock()
hwclock: for debugging print startup system time
sys-utils/hwclock.c | 182 ++++++++++++++++++----------------------------------
1 file changed, 61 insertions(+), 121 deletions(-)
>
> Karel
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] hwclock: rename/refactor set_system_clock()
2017-08-24 23:35 ` J William Piggott
@ 2017-08-25 6:48 ` Karel Zak
0 siblings, 0 replies; 12+ messages in thread
From: Karel Zak @ 2017-08-25 6:48 UTC (permalink / raw)
To: J William Piggott; +Cc: util-linux
On Thu, Aug 24, 2017 at 07:35:35PM -0400, J William Piggott wrote:
> J William Piggott (6):
> hwclock: move systz into hctosys
> hwclock: remove set_system_clock_timezone()
> hwclock: update set_system_clock debugging
> hwclock: update set_system_clock comments
> hwclock: refactor set_system_clock()
> hwclock: for debugging print startup system time
>
> sys-utils/hwclock.c | 182 ++++++++++++++++++----------------------------------
> 1 file changed, 61 insertions(+), 121 deletions(-)
Applied, thanks!
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-08-25 6:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-19 0:16 [PATCH 0/6] Pull Request J William Piggott
2017-08-19 0:18 ` [PATCH 1/6] hwclock: move systz into hctosys J William Piggott
2017-08-19 0:19 ` [PATCH 2/6] hwclock: remove set_system_clock_timezone() J William Piggott
2017-08-19 0:20 ` [PATCH 3/6] hwclock: update set_system_clock debugging J William Piggott
2017-08-19 0:22 ` [PATCH 4/6] hwclock: update set_system_clock comments J William Piggott
2017-08-19 0:23 ` [PATCH 5/6] hwclock: rename/refactor set_system_clock() J William Piggott
2017-08-21 9:31 ` Karel Zak
2017-08-21 12:56 ` J William Piggott
2017-08-22 9:04 ` Karel Zak
2017-08-24 23:35 ` J William Piggott
2017-08-25 6:48 ` Karel Zak
2017-08-19 0:24 ` [PATCH 6/6] hwclock: for debugging print startup system time J William Piggott
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).