[PATCH 0/2] Pull Request

[PATCH 0/2] Pull Request

[J William Piggott]

The following changes since commit 75f03badd7ed9f1dd951863d75e756883d3acc55:

  bash-completion: (umount) use findmnt, escape a space in paths (2017-11-16 16:27:32 +0100)

are available in the git repository at:

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

for you to fetch changes up to af54dbd929266f6706a34050ed51e5114f520f59:

  hwclock: fix debug regression (2017-11-17 20:25:40 -0500)

----------------------------------------------------------------
J William Piggott (2):
      hwclock: use reentrant time conversion functions
      hwclock: fix debug regression

 sys-utils/hwclock.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

[PATCH 1/2] hwclock: use reentrant time conversion functions

[J William Piggott]

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 sys-utils/hwclock.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index c93a5fd65..8ff33474e 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -375,9 +375,9 @@ set_hardware_clock(const struct hwclock_control *ctl, const time_t newtime)
 	 */
 
 	if (ctl->universal)
-		new_broken_time = *gmtime(&newtime);
+		gmtime_r(&newtime, &new_broken_time);
 	else
-		new_broken_time = *localtime(&newtime);
+		localtime_r(&newtime, &new_broken_time);
 
 	if (ctl->debug)
 		printf(_("Setting Hardware Clock to %.2d:%.2d:%.2d "
@@ -600,13 +600,13 @@ static int
 set_system_clock(const struct hwclock_control *ctl,
 		 const struct timeval newtime)
 {
-	struct tm *broken;
+	struct tm broken;
 	int minuteswest;
 	int rc = 0;
 	const struct timezone tz_utc = { 0 };
 
-	broken = localtime(&newtime.tv_sec);
-	minuteswest = -get_gmtoff(broken) / 60;
+	localtime_r(&newtime.tv_sec, &broken);
+	minuteswest = -get_gmtoff(&broken) / 60;
 
 	if (ctl->debug) {
 		if (ctl->hctosys && !ctl->universal)

[PATCH 2/2] hwclock: fix debug regression

[J William Piggott]

commit f0a0ce7 makes debug mode implicit for the --test option.

Using the previous command syntax of --test and --debug
together invokes the undocumented Level 2 debugging when
setting the RTC. This can cause many thousands of lines of
output like:
1510967983.499968 < 1510967983.500000 (-0.000032)

Fix: bump Level 2 debugging to Level 9, just before the
other undocumented Level 10. This makes it improbable for
the development debug levels to be accessed unintentionally.

Signed-off-by: J William Piggott <elseifthen@gmx.com>
---
 sys-utils/hwclock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index 8ff33474e..e5f90a44e 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -494,7 +494,7 @@ set_hardware_clock_exact(const struct hwclock_control *ctl,
 			/* The retarget is handled at the end of the loop. */
 		} else if (deltavstarget < 0) {
 			/* deltavstarget < 0 if current time < target time */
-			if (ctl->debug >= 2)
+			if (ctl->debug >= 9)
 				printf(_("%ld.%06ld < %ld.%06ld (%.6f)\n"),
 				       nowsystime.tv_sec,
 				       nowsystime.tv_usec,

Re: [PATCH 2/2] hwclock: fix debug regression

[Sami Kerola]

On 18 November 2017 at 21:21, J William Piggott <elseifthen@gmx.com> wrote:
>
> commit f0a0ce7 makes debug mode implicit for the --test option.
>
> Using the previous command syntax of --test and --debug
> together invokes the undocumented Level 2 debugging when
> setting the RTC. This can cause many thousands of lines of
> output like:
> 1510967983.499968 < 1510967983.500000 (-0.000032)
>
> Fix: bump Level 2 debugging to Level 9, just before the
> other undocumented Level 10. This makes it improbable for
> the development debug levels to be accessed unintentionally.
>
> Signed-off-by: J William Piggott <elseifthen@gmx.com>
> ---
>  sys-utils/hwclock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
> index 8ff33474e..e5f90a44e 100644
> --- a/sys-utils/hwclock.c
> +++ b/sys-utils/hwclock.c
> @@ -494,7 +494,7 @@ set_hardware_clock_exact(const struct hwclock_control *ctl,
>                         /* The retarget is handled at the end of the loop. */
>                 } else if (deltavstarget < 0) {
>                         /* deltavstarget < 0 if current time < target time */
> -                       if (ctl->debug >= 2)
> +                       if (ctl->debug >= 9)
>                                 printf(_("%ld.%06ld < %ld.%06ld (%.6f)\n"),
>                                        nowsystime.tv_sec,
>                                        nowsystime.tv_usec,

That is probably fine as intermediate issue fix. It would be nice to
see in longer term move to use debug.h. Then ctl->debug could
be made single bit switch that turns on messaging it does, and
all internal developer debugging stuff would be done with
HWCLOCK_DEBUG_ bits. Assuming done this way adding more
debuging levels is appropriate.

FYI login-utils/su-common.c is pretty good DBG example.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [PATCH 2/2] hwclock: fix debug regression

[J William Piggott]

On 11/19/2017 04:51 AM, Sami Kerola wrote:
> That is probably fine as intermediate issue fix. It would be nice to
> see in longer term move to use debug.h. Then ctl->debug could
> be made single bit switch that turns on messaging it does, and
> all internal developer debugging stuff would be done with
> HWCLOCK_DEBUG_ bits. Assuming done this way adding more
> debuging levels is appropriate.
> 
> FYI login-utils/su-common.c is pretty good DBG example.
>

Hi Sami, I agree and so does Karel. He brought it up when I first
started contributing to ul back in 2014.

 https://marc.info/?l=util-linux-ng&m=141415311703313&w=2
 Karel Zak wrote:
>    Good point, then it would be better to [keep] --debug independent on
>    HWCLOCK_DEBUG=. 
>    
>    But I still think that for so complex tool like hwclock we need a 
>    real debug output for developers / bug reporters. The current --debug 
>    is more like --verbose we have in another tools.

It's on my radar and TODO list. I just do not know when I will get to
it. I've been trying to reduce the current 'debug' output as I go.
Hopefully that will make splitting verbose/debug and adding debug.h easier.

Re: [PATCH 2/2] hwclock: fix debug regression

[Karel Zak]

On Sun, Nov 19, 2017 at 03:09:43PM -0500, J William Piggott wrote:
> 
> 
> On 11/19/2017 04:51 AM, Sami Kerola wrote:
> > That is probably fine as intermediate issue fix. It would be nice to
> > see in longer term move to use debug.h. Then ctl->debug could
> > be made single bit switch that turns on messaging it does, and
> > all internal developer debugging stuff would be done with
> > HWCLOCK_DEBUG_ bits. Assuming done this way adding more
> > debuging levels is appropriate.
> > 
> > FYI login-utils/su-common.c is pretty good DBG example.
> >
> 
> Hi Sami, I agree and so does Karel. He brought it up when I first
> started contributing to ul back in 2014.
> 
>  https://marc.info/?l=util-linux-ng&m=141415311703313&w=2
>  Karel Zak wrote:
> >    Good point, then it would be better to [keep] --debug independent on
> >    HWCLOCK_DEBUG=. 
> >    
> >    But I still think that for so complex tool like hwclock we need a 
> >    real debug output for developers / bug reporters. The current --debug 
> >    is more like --verbose we have in another tools.
> 
> It's on my radar and TODO list. I just do not know when I will get to
> it. I've been trying to reduce the current 'debug' output as I go.
> Hopefully that will make splitting verbose/debug and adding debug.h easier.

Yes, the --debug is just bad option name, it should be --verbose
and use HWCLOCK_DEBUG= for the real debugs stuff.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 0/2] Pull Request

[Karel Zak]

On Sat, Nov 18, 2017 at 04:19:15PM -0500, J William Piggott wrote:
> J William Piggott (2):
>       hwclock: use reentrant time conversion functions
>       hwclock: fix debug regression

Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com