Re: readutmp, boot-time: Use the BSD sysctl as fallback

[Bruno Haible]

Pádraig Brady wrote:
> I notice coreutils CI now failing with:
> 
>In file included from lib/readutmp.c:47:
>/usr/include/x86_64-linux-gnu/sys/sysctl.h:21:2: error:
>#warning "The  header is deprecated and will be removed." 
> [-Werror=cpp]
> 21 | #warning "The  header is deprecated and will be 
> removed."
>|  ^~~
> 
> This deprecation of sysctl on glibc >= 2.30 was dealt with in coreutils 
> before like:
> https://github.com/coreutils/coreutils/commit/18c938280
> Perhaps we should avoid this fallback similarly if defined __GLIBC__ ?

I see. Thanks; I did not know about this #warning.

'#if defined __GLIBC__' is not right, because it disables the use of the
sysctl call also on GNU/kFreeBSD. Per glibc commit
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=076f09afbac1aa57756faa7a8feadb7936a724e4
the right condition to use is '#if defined __GLIBC__ && defined __linux__'.

This patch fixes it. Tested on Fedora 31 (glibc 2.30) and openSUSE 15.5 (glibc 
2.31).


2023-08-13  Bruno Haible  

readutmp, boot-time: Fix warning on glibc 2.30..2.31 on Linux.
Reported by Pádraig Brady in
.
* lib/readutmp.c: Don't include  on glibc/Linux.
* lib/boot-time.c: Likewise.

diff --git a/lib/boot-time.c b/lib/boot-time.c
index c359954f19..fe5b5b88c8 100644
--- a/lib/boot-time.c
+++ b/lib/boot-time.c
@@ -32,7 +32,7 @@
 # include 
 #endif
 
-#if HAVE_SYS_SYSCTL_H && !defined __minix
+#if HAVE_SYS_SYSCTL_H && !(defined __GLIBC__ && defined __linux__) && !defined 
__minix
 # if HAVE_SYS_PARAM_H
 #  include 
 # endif
diff --git a/lib/readutmp.c b/lib/readutmp.c
index 0b7732b165..7967db60a8 100644
--- a/lib/readutmp.c
+++ b/lib/readutmp.c
@@ -40,7 +40,7 @@
 # include 
 #endif
 
-#if HAVE_SYS_SYSCTL_H && !defined __minix
+#if HAVE_SYS_SYSCTL_H && !(defined __GLIBC__ && defined __linux__) && !defined 
__minix
 # if HAVE_SYS_PARAM_H
 #  include 
 # endif

Re: readutmp, boot-time: Use the BSD sysctl as fallback

[Pádraig Brady]

On 12/08/2023 14:22, Bruno Haible wrote:

The best approaches to get the boot time access some files. For the
case of programs that execute in Docker containers on Linux, we have
the fallback that relies on the kernel's uptime counter — although it
produces wrong values in a VM that has been put to sleep and then resumed.

Similarly, it is useful to have a fallback for BSD 'jails' on *BSD systems.
There is a sysctl named KERN_BOOTTIME, that coreutils/src/uptime.c already
uses. For consistency between 'who' and 'uptime', it is good to have this
fallback in the 'readutmp' module.

This sysctl exists on macOS, FreeBSD, NetBSD, OpenBSD, GNU/kFreeBSD, Minix.
But note:
   - The boot time that this sysctl reports changes after a VM has been
 put to sleep, then resumed, then its 'date' changed. I've verified
 this on FreeBSD, NetBSD, OpenBSD, GNU/kFreeBSD. It's probably the
 same thing on macOS (but I can't test it).
   - On Minix 3.3, the result of this system call is garbage. When I compile
 the attached program, it produces completely different results when
 invoked as
   $ ./a.out
 vs.
   $ TZ=UTC ./a.out



I notice coreutils CI now failing with:

  In file included from lib/readutmp.c:47:
  /usr/include/x86_64-linux-gnu/sys/sysctl.h:21:2: error:
  #warning "The  header is deprecated and will be removed." 
[-Werror=cpp]
   21 | #warning "The  header is deprecated and will be removed."
  |  ^~~

This deprecation of sysctl on glibc >= 2.30 was dealt with in coreutils before 
like:
https://github.com/coreutils/coreutils/commit/18c938280
Perhaps we should avoid this fallback similarly if defined __GLIBC__ ?

thanks,
Pádraig

readutmp, boot-time: Use the BSD sysctl as fallback

[Bruno Haible]

The best approaches to get the boot time access some files. For the
case of programs that execute in Docker containers on Linux, we have
the fallback that relies on the kernel's uptime counter — although it
produces wrong values in a VM that has been put to sleep and then resumed.

Similarly, it is useful to have a fallback for BSD 'jails' on *BSD systems.
There is a sysctl named KERN_BOOTTIME, that coreutils/src/uptime.c already
uses. For consistency between 'who' and 'uptime', it is good to have this
fallback in the 'readutmp' module.

This sysctl exists on macOS, FreeBSD, NetBSD, OpenBSD, GNU/kFreeBSD, Minix.
But note:
  - The boot time that this sysctl reports changes after a VM has been
put to sleep, then resumed, then its 'date' changed. I've verified
this on FreeBSD, NetBSD, OpenBSD, GNU/kFreeBSD. It's probably the
same thing on macOS (but I can't test it).
  - On Minix 3.3, the result of this system call is garbage. When I compile
the attached program, it produces completely different results when
invoked as
  $ ./a.out
vs.
  $ TZ=UTC ./a.out


2023-08-12  Bruno Haible  

readutmp, boot-time: Use the BSD sysctl as fallback.
* m4/readutmp.m4 (gl_PREREQ_READUTMP_H): Test for ,
, sysctl.
* lib/boot-time-aux.h (get_bsd_boot_time_final_fallback): New function.
* lib/readutmp.c: Include  and .
(read_utmp_from_file): Invoke get_bsd_boot_time_final_fallback as a
fallback.
* lib/boot-time.c: Include  and .
(get_boot_time_uncached): Invoke get_bsd_boot_time_final_fallback as a
fallback.

diff --git a/lib/boot-time-aux.h b/lib/boot-time-aux.h
index fc84086f3f..8de834efd7 100644
--- a/lib/boot-time-aux.h
+++ b/lib/boot-time-aux.h
@@ -199,7 +199,35 @@ get_openbsd_boot_time (struct timespec *p_boot_time)
 
 #endif
 
-# if defined __CYGWIN__ || defined _WIN32
+#if HAVE_SYS_SYSCTL_H && HAVE_SYSCTL \
+&& defined CTL_KERN && defined KERN_BOOTTIME \
+&& !defined __minix
+/* macOS, FreeBSD, GNU/kFreeBSD, NetBSD, OpenBSD */
+/* On Minix 3.3 this sysctl produces garbage results.  Therefore avoid it.  */
+
+/* The following approach is only usable as a fallback, because it produces
+   wrong values after the date has been bumped in the running system, which
+   happens frequently if the system is running in a virtual machine and this
+   VM has been put into "saved" or "sleep" state and then resumed.  */
+static int
+get_bsd_boot_time_final_fallback (struct timespec *p_boot_time)
+{
+  static int request[2] = { CTL_KERN, KERN_BOOTTIME };
+  struct timeval result;
+  size_t result_len = sizeof result;
+
+  if (sysctl (request, 2, &result, &result_len, NULL, 0) >= 0)
+{
+  p_boot_time->tv_sec = result.tv_sec;
+  p_boot_time->tv_nsec = result.tv_usec * 1000;
+  return 0;
+}
+  return -1;
+}
+
+#endif
+
+#if defined __CYGWIN__ || defined _WIN32
 
 static int
 get_windows_boot_time (struct timespec *p_boot_time)
@@ -225,4 +253,4 @@ get_windows_boot_time (struct timespec *p_boot_time)
   return -1;
 }
 
-# endif
+#endif
diff --git a/lib/boot-time.c b/lib/boot-time.c
index 932ee22d69..b27cb6c6b6 100644
--- a/lib/boot-time.c
+++ b/lib/boot-time.c
@@ -32,6 +32,13 @@
 # include 
 #endif
 
+#if HAVE_SYS_SYSCTL_H && !defined __minix
+# if HAVE_SYS_PARAM_H
+#  include 
+# endif
+# include 
+#endif
+
 #include "idx.h"
 #include "readutmp.h"
 #include "stat-time.h"
@@ -66,14 +73,7 @@ get_boot_time_uncached (struct timespec *p_boot_time)
 {
   struct timespec found_boot_time = {0};
 
-# if !(HAVE_UTMPX_H ? HAVE_STRUCT_UTMPX_UT_TYPE : HAVE_STRUCT_UTMP_UT_TYPE) /* 
old FreeBSD, OpenBSD, native Windows */
-
-#  if defined __OpenBSD__
-  /* Workaround for OpenBSD:  */
-  get_openbsd_boot_time (&found_boot_time);
-#  endif
-
-# else
+# if (HAVE_UTMPX_H ? HAVE_STRUCT_UTMPX_UT_TYPE : HAVE_STRUCT_UTMP_UT_TYPE)
 
   /* Try to find the boot time in the /var/run/utmp file.  */
 
@@ -198,6 +198,22 @@ get_boot_time_uncached (struct timespec *p_boot_time)
 }
 #  endif
 
+# else /* old FreeBSD, OpenBSD, native Windows */
+
+#  if defined __OpenBSD__
+  /* Workaround for OpenBSD:  */
+  get_openbsd_boot_time (&found_boot_time);
+#  endif
+
+# endif
+
+# if HAVE_SYS_SYSCTL_H && HAVE_SYSCTL \
+ && defined CTL_KERN && defined KERN_BOOTTIME \
+ && !defined __minix
+  if (found_boot_time.tv_sec == 0)
+{
+  get_bsd_boot_time_final_fallback (&found_boot_time);
+}
 # endif
 
 # if defined __CYGWIN__ || defined _WIN32
diff --git a/lib/readutmp.c b/lib/readutmp.c
index d7baa69909..9a5b34054a 100644
--- a/lib/readutmp.c
+++ b/lib/readutmp.c
@@ -40,6 +40,13 @@
 # include 
 #endif
 
+#if HAVE_SYS_SYSCTL_H && !defined __minix
+# if HAVE_SYS_PARAM_H
+#  include 
+# end