bug#70477: tail command does not hang on /dev/random

[Paul Eggert]

On 2024-04-19 11:40, Ionut Nicula wrote:

The following command hangs indefinitely, as expected:

 tail -c 4096 /dev/random

The following command immediately returns 4097 bytes, unexpectedly:

 tail -c 4097 /dev/random


Thanks for the bug report. Although the two commands should behave 
similarly, neither should loop: they should both output the requested 
number of random bytes. Similarly for /dev/zero. I installed the 
attached patch to do that.From fb543b6b82c1f3a20ff88f44cc3ed367bfe811b6 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 19 Apr 2024 21:44:32 -0700
Subject: [PATCH] tail: avoid infloop with -c on /dev/zero

Problem reported by Ionut Nicula in:
https://bugs.gnu.org/70477
* src/tail.c (tail_bytes): Do not loop forever on commands
like 'tail -c 4096 /dev/zero'.
* tests/tail/tail-c.sh: Test this fix.
---
 NEWS |  3 +++
 src/tail.c   | 24 +++-
 tests/tail/tail-c.sh | 10 ++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 43ce84d7e..389f72516 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,9 @@ GNU coreutils NEWS-*- outline -*-
   have exited with a "Function not implemented" error.
   [bug introduced in coreutils-8.28]
 
+  'tail -c 4096 /dev/zero' no longer loops forever.
+  [This bug was present in "the beginning".]
+
 ** Changes in behavior
 
   ls's -f option now simply acts like -aU, instead of also ignoring
diff --git a/src/tail.c b/src/tail.c
index 52c081031..a3b46ca2d 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -760,7 +760,8 @@ free_lbuffers:
   return ok;
 }
 
-/* Print the last N_BYTES characters from the end of pipe FD.
+/* Print the last N_BYTES characters from the end of FD.
+   Work even if the input is a pipe.
This is a stripped down version of pipe_lines.
Return true if successful.  */
 
@@ -1875,15 +1876,28 @@ tail_bytes (char const *pretty_filename, int fd, uintmax_t n_bytes,
 {
   off_t end_pos = -1;
   off_t current_pos = -1;
+  bool copy_from_current_pos = false;
 
   if (! presume_input_pipe && n_bytes <= OFF_T_MAX)
 {
   if (usable_st_size ())
-end_pos = stats.st_size;
-  else if ((current_pos = lseek (fd, -n_bytes, SEEK_END)) != -1)
-end_pos = current_pos + n_bytes;
+{
+  /* Use st_size only if it's so large that this is
+ probably not a /proc or similar file, where st_size
+ is notional.  */
+  end_pos = stats.st_size;
+  off_t smallish_size = STP_BLKSIZE ();
+  copy_from_current_pos = smallish_size < end_pos;
+}
+  else
+{
+  current_pos = lseek (fd, -n_bytes, SEEK_END);
+  copy_from_current_pos = current_pos != -1;
+  if (copy_from_current_pos)
+end_pos = current_pos + n_bytes;
+}
 }
-  if (end_pos <= (off_t) STP_BLKSIZE ())
+  if (! copy_from_current_pos)
 return pipe_bytes (pretty_filename, fd, n_bytes, read_pos);
   if (current_pos == -1)
 current_pos = xlseek (fd, 0, SEEK_CUR, pretty_filename);
diff --git a/tests/tail/tail-c.sh b/tests/tail/tail-c.sh
index f518e5b21..a9f2bc2d1 100755
--- a/tests/tail/tail-c.sh
+++ b/tests/tail/tail-c.sh
@@ -35,4 +35,14 @@ printf '123456' | tail -c3 > out || fail=1
 printf '456' > exp || framework_failure_
 compare exp out || fail=1
 
+# Any part of /dev/zero should be valid for tail -c.
+head -c 4096 /dev/zero >exp || fail=1
+tail -c 4096 /dev/zero >out || fail=1
+compare exp out || fail=1
+
+# Any part of /dev/urandom, if it exists, should be valid for tail -c.
+if test -r /dev/urandom; then
+  timeout --verbose 1 tail -c 4096 /dev/urandom >/dev/null || fail=1
+fi
+
 Exit $fail
-- 
2.40.1

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 2024-04-18 14:52, Sergei Trofimovich wrote:

$ clang simple.c -o simple && echo 42 | ./simple
1: ino=3009428657538693161
2: ino=3009428657538693161
3: ino=1568241705

Note how stat() and fstat() don't agree on inode.

Apparently it's documented in
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html
as

   BUGS
  Applying fstat to a socket (and thus to a pipe) returns a zero'd buffer,
  except for the blocksize field, and a unique device and inode number.


The BUGS note simply means that a pipe has a unique inode number, which 
is what we want. So that's not indicating any problem.



Oh, I see the problem now.  For a socket or pipe, macOS fstat returns 
the full 64-bit inode number, whereas macOS stat returns only the low 
order 32 bits.  In your example, 3009428657538693161 % (2**32) == 
1568241705.


This is a kernel bug in macOS. Can you report it or otherwise arrange to 
have the kernel bug fixed? I expect that you have better connections 
with Apple than I do. A proposed patch (relative to xnu-10063.101.15) is 
attached; I have not tested it as I don't use macOS. Thanks.


Also, I am documenting this macOS bug in Gnulib by installing the second 
attached patch to Gnulib, and am cc'ing this email to bug-gnulib.From 29345117a4cf85aceb88e3901758b19a4867062e Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 19 Apr 2024 00:12:50 -0700
Subject: [PATCH] Fix bug with stat truncating pipe/socket st_ino

Problem reported by Sergei Trofimovich in:
https://bugs.gnu.org/70411
https://github.com/NixOS/nixpkgs/pull/300797
* bsd/miscfs/devfs/devfs_fdesc_support.c (fdesc_attr):
Do not truncate inode numbers to 32 bits.
---
 bsd/miscfs/devfs/devfs_fdesc_support.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bsd/miscfs/devfs/devfs_fdesc_support.c b/bsd/miscfs/devfs/devfs_fdesc_support.c
index a17c6992..b4a55103 100644
--- a/bsd/miscfs/devfs/devfs_fdesc_support.c
+++ b/bsd/miscfs/devfs/devfs_fdesc_support.c
@@ -437,10 +437,10 @@ fdesc_attr(int fd, struct vnode_attr *vap, vfs_context_t a_context)
 	case DTYPE_PIPE:
 #if SOCKETS
 		if (FILEGLOB_DTYPE(fp->fp_glob) == DTYPE_SOCKET) {
-			error = soo_stat((struct socket *)fp_get_data(fp), (void *), 0);
+			error = soo_stat((struct socket *)fp_get_data(fp), (void *), 1);
 		} else
 #endif /* SOCKETS */
-		error = pipe_stat((struct pipe *)fp_get_data(fp), (void *), 0);
+		error = pipe_stat((struct pipe *)fp_get_data(fp), (void *), 1);
 
 		if (error == 0) {
 			if (FILEGLOB_DTYPE(fp->fp_glob) == DTYPE_SOCKET) {
-- 
2.44.0

From c2174a623d33096b52f4d7fd2963f76acb3e301f Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 19 Apr 2024 00:29:32 -0700
Subject: [PATCH] fstatat, stat: document macOS st_ino pipe bug

* doc/posix-functions/fstatat.texi (fstatat):
* doc/posix-functions/stat.texi (stat):
Document macOS bug (see <https://bugs.gnu.org/70411>).
---
 ChangeLog| 7 +++
 doc/posix-functions/fstatat.texi | 5 +
 doc/posix-functions/stat.texi| 5 +
 3 files changed, 17 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 7ce75a98a9..1667f90c55 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-04-19  Paul Eggert  
+
+	fstatat, stat: document macOS st_ino pipe bug
+	* doc/posix-functions/fstatat.texi (fstatat):
+	* doc/posix-functions/stat.texi (stat):
+	Document macOS bug (see <https://bugs.gnu.org/70411>).
+
 2024-04-18  Bruno Haible  
 
 	totalordermagl: Add tests.
diff --git a/doc/posix-functions/fstatat.texi b/doc/posix-functions/fstatat.texi
index e959a5cc73..90884e2eb1 100644
--- a/doc/posix-functions/fstatat.texi
+++ b/doc/posix-functions/fstatat.texi
@@ -40,5 +40,10 @@ This function does not fail when the second argument is an empty string
 on some platforms, even when @code{AT_EMPTY_PATH} is not used:
 glibc 2.7, Linux 2.6.38.
 @item
+This function sets @code{st_ino} only to the low-order 32 bits of
+the inode number of a socket or pipe, which thus can disagree
+with the @code{st_ino} obtained by @code{fstat}:
+macOS 14.
+@item
 @xref{sys/stat.h}, for general portability problems with @code{struct stat}.
 @end itemize
diff --git a/doc/posix-functions/stat.texi b/doc/posix-functions/stat.texi
index f655451392..8afd3b17bb 100644
--- a/doc/posix-functions/stat.texi
+++ b/doc/posix-functions/stat.texi
@@ -50,6 +50,11 @@ Portability problems not fixed by Gnulib:
 Cygwin's @code{stat} function sometimes sets @code{errno} to @code{EACCES} when
 @code{ENOENT} would be more appropriate.
 @item
+This function sets @code{st_ino} only to the low-order 32 bits of
+the inode number of a socket or pipe, which thus can disagree
+with the @code{st_ino} obtained by @code{fstat}:
+macOS 14.
+@item
 Because of the definition of @code{struct stat}, it is not possible to
 portably replace @code{stat} via an object-like macro.  Therefore,
 expressions such as @code{(islnk ? lstat : stat) (name, buf)} are not
-- 
2.40.1

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 4/18/24 14:52, Sergei Trofimovich wrote:


https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html
as

   BUGS
  Applying fstat to a socket (and thus to a pipe) returns a zero'd buffer,
  except for the blocksize field, and a unique device and inode number.


How is a macOS program supposed to tell whether a file has been changed 
between stat and open? Is it impossible?

bug#70418: ls bug

[Paul Eggert]

On 4/17/24 03:19, Pádraig Brady wrote:


Patch looks good, and conforms to POSIX.


Thanks for the review. I installed the patch and am closing this bug report.

bug#70418: ls bug

[Paul Eggert]

On 4/16/24 14:30, Toby Kelsey wrote:
The man page doesn't explain this format conflict, while the info page 
(info '(coreutils) ls invocation' or 'info ls') claims '-f' implies '-1' 
which is also incorrect: 'ls -1f' gives different output to to 'ls -f'.


Yes, this area of GNU 'ls' a mess. Option order should not matter here.

Option order didn't matter in 7th Edition Unix, where -f overrode -l 
regardless of whether -f came before or after -l. And option order 
doesn't matter in FreeBSD, where -f and -l are orthogonal. GNU ls is an 
odd hybrid of 7th Edition and FreeBSD and messes this up.


Rather than document the hybrid mess, let's bite the bullet and fix it. 
FreeBSD behavior makes more sense, so let's do that. Proposed patch 
attached.From 674a6c262e594102b0485c5ded854c1be5902777 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 16 Apr 2024 15:08:51 -0700
Subject: [PATCH] ls: -f now means -a -U

Problem reported by Toby Kelsey <https://bugs.gnu.org/70418>.
* src/ls.c (decode_switches): -f now simply means -a -U.
(usage): Adjust to match.
---
 NEWS   |  7 +++
 doc/coreutils.texi | 14 +++---
 src/ls.c   |  7 +--
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index 21ed3435b..43ce84d7e 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,13 @@ GNU coreutils NEWS-*- outline -*-
   have exited with a "Function not implemented" error.
   [bug introduced in coreutils-8.28]
 
+** Changes in behavior
+
+  ls's -f option now simply acts like -aU, instead of also ignoring
+  some earlier options.  For example 'ls -fl' and 'ls -lf' are now
+  equivalent because -f no longer ignores an earlier -l.  The new
+  behavior is more orthogonal and is compatible with FreeBSD.
+
 ** Improvements
 
   sort operates more efficiently when used on pseudo files with
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 8f7f43ea0..2c6c8b68b 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8150,11 +8150,9 @@ sort according to the ctime.  @xref{File timestamps}.
 @opindex -f
 @cindex unsorted directory listing
 @cindex directory order, listing by
-Produce an unsorted directory listing.
-This is like @option{--sort=none} (@option{-U}),
-but also enable @option{--all} (@option{-a}),
-while also disabling any previous use of @option{-l}, @option{--color}
-@option{--size}, or @option{--hyperlink}.
+Produce an unsorted listing of all directory entries.
+This is like @option{--all} (@option{-a}) combined
+with @option{--sort=none} (@option{-U}).
 
 @item -r
 @itemx --reverse
@@ -8224,10 +8222,12 @@ sort according to the birth time.
 @opindex --sort
 @opindex none@r{, sorting option for @command{ls}}
 Do not sort; list the files in whatever order they are
-stored in the directory.  (Do not do any of the other unrelated things
-that @option{-f} does.)  This can be useful when listing large
+stored in the directory.  This can be useful when listing large
 directories, where sorting can take some time.
 
+Unlike @option{-f}, this option does not imply @option{--all}
+(@option{-a}).
+
 @item -v
 @itemx --sort=version
 @opindex -v
diff --git a/src/ls.c b/src/ls.c
index f5ac98df7..916d7c050 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1952,11 +1952,6 @@ decode_switches (int argc, char **argv)
 case 'f':
   ignore_mode = IGNORE_MINIMAL; /* enable -a */
   sort_opt = sort_none; /* enable -U */
-  if (format_opt == long_format)
-format_opt = -1;/* disable -l */
-  print_with_color = false; /* disable --color */
-  print_hyperlink = false;  /* disable --hyperlink */
-  print_block_size = false; /* disable -s */
   break;
 
 case FILE_TYPE_INDICATOR_OPTION: /* --file-type */
@@ -5466,7 +5461,7 @@ Sort entries alphabetically if none of -cftuvSUX nor --sort is specified.\n\
   -D, --diredgenerate output designed for Emacs' dired mode\n\
 "), stdout);
   fputs (_("\
-  -f do not sort, enable -aU, disable -ls --color\n\
+  -f same as -a -U\n\
   -F, --classify[=WHEN]  append indicator (one of */=>@|) to entries WHEN\n\
   --file-typelikewise, except do not append '*'\n\
 "), stdout);
-- 
2.44.0

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 4/16/24 12:44, Pádraig Brady wrote:

A related suggestion was from Marc Chantreux (CC'd)
to support '-' to imply stdin, which would be more portable.
There is some merit to that suggestion too.


I see that merit too, as when 'install' reads from stdin it needn't do 
the inode check. However, POSIX requires 'cp - X' to copy a file named 
'-' to X; cp cannot copy from stdin. Although POSIX doesn't specify 
'install', there is some value to having 'install' behave like 'cp'.


I suppose we could have install special-case "/dev/stdin", on platforms 
where /dev/stdin is known to mean standard input. Seems like a bit much 
to work around a Darwin kernel bug, though.

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 4/16/24 07:47, Alejandro Colomar wrote:

Since you couldn't reprodude it in a recent Darwin, maybe it's
just a bug in an old Darwin.


It'd have to be pretty old. As near as I can see from 
xnu/bsd/kern/sys_pipe.c, the st_ino field was zero (i.e., not random) 
even in xnu-792 dated 2005.


I'd be leery of changing this part of coreutils/src/copy.c without 
knowing more about exactly why bug occurs on (older?) Darwin, as 
weakening the check would make GNU 'install' easier to attack.

bug#70298: uname -i returns unknown since fedora 38

[Paul Eggert]

On 2024-04-08 10:59, Ondrej Mejzlik wrote:

The command uname returns "unknown" on Fedora 38,39 and probably 40.
There is a similar very old bug here which has not been fixed
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34905


Indeed there is, and I merged your bug report into that old one. It'd be 
nice if someone could get to the bottom of that bug.

bug#70231: Performance issue on sort with zero-sized pseudo files

[Paul Eggert]

On 2024-04-06 03:09, Pádraig Brady wrote:

I'll apply this.


Heh, I beat you to it by looking for similar errors elsewhere and 
applying the attached patches to fix the issues I found. None of them 
look like serious bugs.



BTW we should improve sort buffer handling in general


Oh yes.

PS. My current little task is to get i18n to work better with 'sort'. 
Among other things I want Unicode-style full case folding.From 225cb8d7473eadb481a4884e929bf23589d4bd82 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 6 Apr 2024 15:13:23 -0700
Subject: [PATCH 1/4] =?UTF-8?q?cat:=20don=E2=80=99t=20trust=20st=5Fsize=20?=
 =?UTF-8?q?on=20/proc=20files?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/cat.c (main):
Improve test for when copying will exhaust the output device.
Do not rely on st_size, which is unreliable in /proc.
Use lseek instead; this is good enough here.
* tests/cat/cat-self.sh: Test the relaxation of the heuristic
for self-copying.
---
 src/cat.c | 31 +--
 tests/cat/cat-self.sh | 20 
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/src/cat.c b/src/cat.c
index 4ed404363..b33faeb35 100644
--- a/src/cat.c
+++ b/src/cat.c
@@ -645,9 +645,10 @@ main (int argc, char **argv)
   /* Optimal size of i/o operations of output.  */
   idx_t outsize = io_blksize (_buf);
 
-  /* Device and I-node number of the output.  */
+  /* Device, I-node number and lazily-acquired flags of the output.  */
   dev_t out_dev = stat_buf.st_dev;
   ino_t out_ino = stat_buf.st_ino;
+  int out_flags = -2;
 
   /* True if the output is a regular file.  */
   bool out_isreg = S_ISREG (stat_buf.st_mode) != 0;
@@ -701,17 +702,27 @@ main (int argc, char **argv)
 
   fdadvise (input_desc, 0, 0, FADVISE_SEQUENTIAL);
 
-  /* Don't copy a nonempty regular file to itself, as that would
- merely exhaust the output device.  It's better to catch this
- error earlier rather than later.  */
+  /* Don't copy a file to itself if that would merely exhaust the
+ output device.  It's better to catch this error earlier
+ rather than later.  */
 
-  if (out_isreg
-  && stat_buf.st_dev == out_dev && stat_buf.st_ino == out_ino
-  && lseek (input_desc, 0, SEEK_CUR) < stat_buf.st_size)
+  if (stat_buf.st_dev == out_dev && stat_buf.st_ino == out_ino)
 {
-  error (0, 0, _("%s: input file is output file"), quotef (infile));
-  ok = false;
-  goto contin;
+  if (out_flags < -1)
+out_flags = fcntl (STDOUT_FILENO, F_GETFL);
+  bool exhausting = 0 <= out_flags && out_flags & O_APPEND;
+  if (!exhausting)
+{
+  off_t in_pos = lseek (input_desc, 0, SEEK_CUR);
+  if (0 <= in_pos)
+exhausting = in_pos < lseek (STDOUT_FILENO, 0, SEEK_CUR);
+}
+  if (exhausting)
+{
+  error (0, 0, _("%s: input file is output file"), quotef (infile));
+  ok = false;
+  goto contin;
+}
 }
 
   /* Pointer to the input buffer.  */
diff --git a/tests/cat/cat-self.sh b/tests/cat/cat-self.sh
index e0f6455c0..854825def 100755
--- a/tests/cat/cat-self.sh
+++ b/tests/cat/cat-self.sh
@@ -30,4 +30,24 @@ echo y >doc.end || framework_failure_
 cat doc doc.end >doc || fail=1
 compare doc doc.end || fail=1
 
+# This terminates even though it copies a file to itself.
+# Coreutils 9.5 and earlier rejected this.
+echo x >fx || framework_failure_
+echo y >fy || framework_failure_
+cat fx fy >fxy || fail=1
+for i in 1 2; do
+  cat fx >fxy$i || fail=1
+done
+for i in 3 4 5 6; do
+  cat fx >fx$i || fail=1
+done
+cat - fy fxy1 || fail=1
+compare fxy fxy1 || fail=1
+cat fxy2 fy 1<>fxy2 || fail=1
+compare fxy fxy2 || fail=1
+returns_ 1 cat fx fx3 1<>fx3 || fail=1
+returns_ 1 cat - fx4 fx4 || fail=1
+returns_ 1 cat fx5 >>fx5 || fail=1
+returns_ 1 cat >fx6 || fail=1
+
 Exit $fail
-- 
2.40.1

From ac6b8d8224de140f5a6f2ca66e6ce279604a37e6 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 6 Apr 2024 15:15:04 -0700
Subject: [PATCH 2/4] =?UTF-8?q?dd:=20don=E2=80=99t=20trust=20st=5Fsize=20o?=
 =?UTF-8?q?n=20/proc/files?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/dd.c (skip): Don’t trust st_size == 0.
---
 src/dd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dd.c b/src/dd.c
index 370a42c97..6e0d725da 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1809,7 +1809,7 @@ skip (int fdesc, char const *file, intmax_t records, idx_t blocksize,
struct stat st;
if (ifstat (STDIN_FILENO, ) != 0)
  error (EXIT_FAILURE, errno, _("cannot fstat %s"), quoteaf (file

bug#70104: "FAIL: test-canonicalize" coreutils v9.5 on musl libc

[Paul Eggert]

On 2024-03-31 12:45, Bruno Haible wrote:

I think you must ask this to yourself:
   - What caused you to change the unit tests on 2023-09-04?
   - How is the musl version that you used on that date configured?
 What I use is Alpine Linux, as I said in versions 3.9, 3.14, 3.19.
   - Did you work with gnulib testdirs at that time, or did you use
 a package in which some modules are --avoid ed from import?


Thanks for fixing the problem. I cannot now remember why I put that musl 
stuff in. I suspect that I got it from some tests in some other package, 
but don't recall which ones. I don't use musl myself.

bug#70104: "FAIL: test-canonicalize" coreutils v9.5 on musl libc

[Paul Eggert]

On 3/31/24 05:22, Pádraig Brady wrote:

On 31/03/2024 10:02, Adept's Lab wrote:

test-canonicalize.c:411: assertion 'strcmp (result1, "//") == 0' failed

^ the only error log message I get. Fail was not presented with previous
stable versions.


This is on musl 1.1.24 as detailed at:
https://github.com/coreutils/coreutils/issues/83T
Does this behavior (of whether / and // are the same directory) depend 
on musl version, or on how musl is configured?

bug#70070: FAIL: test-localtime_r

[Paul Eggert]

On 3/29/24 08:15, Andreas Schwab wrote:

On Mär 29 2024, Bruno Haible wrote:


Yes. And make sure that it has a time zone database installed at all.


Why? That doesn't make any sense.


Although Andreas is not clear, perhaps he is alluding to the fact that 
Gnulib's localtime_r tests assume that TZ='Europe/Paris' stands for 
Paris time. POSIX doesn't guarantee this, so the implication is that the 
tests should be skipped when Europe/Paris does not work.


I installed the attached to do that. I don't know whether this will fix 
Andreas's problem, whatever it is.From f130f5426ecd4edd5596797e0a5721b927f80126 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 30 Mar 2024 13:28:01 -0600
Subject: [PATCH] time_r-tests: skip French tests if no Europe/Paris

* tests/test-localtime_r.c (main):
* tests/test-localtime_r-mt.c (main):
If TZ='Europe/Paris' does not work, skip these tests.
---
 ChangeLog   |  7 +++
 tests/test-localtime_r-mt.c | 21 +
 tests/test-localtime_r.c| 21 +
 3 files changed, 49 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index eab98607a2..ecb4632196 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-03-30  Paul Eggert  
+
+	time_r-tests: skip French tests if no Europe/Paris
+	* tests/test-localtime_r.c (main):
+	* tests/test-localtime_r-mt.c (main):
+	If TZ='Europe/Paris' does not work, skip these tests.
+
 2024-03-29  Paul Eggert  
 
 	intprops: pacify GCC < 10 -Wsign-compare
diff --git a/tests/test-localtime_r-mt.c b/tests/test-localtime_r-mt.c
index ae371a8946..b09e86854f 100644
--- a/tests/test-localtime_r-mt.c
+++ b/tests/test-localtime_r-mt.c
@@ -107,6 +107,27 @@ main (int argc, char *argv[])
 {
   setenv ("TZ", FRENCH_TZ, 1);
 
+  /* Check that this TZ works.  */
+  {
+time_t t = 0; /* 1970-01-01 01:00:00 */
+struct tm *result = localtime ();
+if (! (result
+   && result->tm_sec == 0
+   && result->tm_min == 0
+   && result->tm_hour == 1
+   && result->tm_mday == 1
+   && result->tm_mon == 1 - 1
+   && result->tm_year == 1970 - 1900
+   && result->tm_wday == 4
+   && result->tm_yday == 0
+   && result->tm_isdst == 0))
+  {
+fputs ("Skipping test: TZ='Europe/Paris' is not Paris time\n",
+   stderr);
+return 77;
+  }
+  }
+
   /* Create the threads.  */
   gl_thread_create (thread1_func, NULL);
   gl_thread_create (thread2_func, NULL);
diff --git a/tests/test-localtime_r.c b/tests/test-localtime_r.c
index 70ec3b5d4f..6b3da1af19 100644
--- a/tests/test-localtime_r.c
+++ b/tests/test-localtime_r.c
@@ -43,6 +43,27 @@ main (void)
 {
   setenv ("TZ", FRENCH_TZ, 1);
 
+  /* Check that this TZ works.  */
+  {
+time_t t = 0; /* 1970-01-01 01:00:00 */
+struct tm *result = localtime ();
+if (! (result
+   && result->tm_sec == 0
+   && result->tm_min == 0
+   && result->tm_hour == 1
+   && result->tm_mday == 1
+   && result->tm_mon == 1 - 1
+   && result->tm_year == 1970 - 1900
+   && result->tm_wday == 4
+   && result->tm_yday == 0
+   && result->tm_isdst == 0))
+  {
+fputs ("Skipping test: TZ='Europe/Paris' is not Paris time\n",
+   stderr);
+return 77;
+  }
+  }
+
   /* Note: The result->tm_gmtoff values and the result->tm_zone values are the
  same (3600, "CET" or 7200, "CEST") across all tested platforms:
  glibc, musl, macOS, FreeBSD, NetBSD, OpenBSD, Minix, Cygwin, Android.  */
-- 
2.44.0

Re: coreutils-9.4.170-7b206 ls/removed-directory test failure

[Paul Eggert]

On 3/26/24 11:35, Pádraig Brady wrote:

Actually the FreeBSD system ls(1) does _not_ show the error in this case
(it doesn't use readdir I think).


If I'm reading the source code aright, FreeBSD ls uses fts, which does 
use readdir. It's not clear to me whether it's intended that the readdir 
errno is swallowed up by fts's maze of twisty code paths.




So I'm not leaning towards GNU ls
just eating the ENOENT which simplifies the test and makes both
ls implementations behave the same in this regard.


I assume you meant "now" not "not". That sounds OK to me, thanks.

Re: coreutils-9.4.170-7b206 ls/removed-directory test failure

[Paul Eggert]

On 3/26/24 10:50, Pádraig Brady wrote:
It seems that readdir() on FreeBSD 14 is _not_ eating the ENOENT from 
getdirentries().

Attached is an extra check for that, that avoids the test in that case.

An alternative would be for ls to ignore the ENOENT from readdir().
Paul what do you think?


My quick reaction is that your patch is good. This is an area where the 
FreeBSD behavior is reasonable and when coreutils is in Rome we might as 
well do what the Romans do.

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/21/24 14:45, Bernhard Voelker wrote:

On 3/21/24 00:56, Paul Eggert wrote:

On 3/20/24 15:53, Bernhard Voelker wrote:
Yes, that's the expected behavior for this contrived case. Just as one
would get odd behavior if one did the same thing without --exchange.


There's another which is not consistent with/without --exchange:

   $ src/mv -v a a
   src/mv: 'a' and 'a' are the same file

   $ src/mv -v --exchange a a
   renamed 'a' -> 'a'

RENAME_EXCHANGE is allowed (but useless?) for 1 file.


Yes, thanks, --exchange should act more like non --exchange there.



BTW: shouldn't the -v diagnostic better say "exchanged 'a' <-> 'a'"
because that's what happened?


Good suggestion.



It seems that -i is skipped:

   $ src/mv -iv --exchange a b
   renamed 'a' -> 'b'


Yes, I suppose -i should be treated similarly too.

I installed the attached patches to do the above. (Basically, the 
problem was that my earlier patches were too ambitious; these patches 
scale things back to avoid some optimizations so that mv --exchange is 
more like ordinary mv.)


The first patch simplifies the code (and fixes a diagnostic to be more 
useful) without otherwise changing behavior; it's more of a refactoring. 
The second patch does the real work.


Thanks again.
From ff42adb55e99226f55a7b141316ee2a7b84a4857 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 22 Mar 2024 12:02:41 -0700
Subject: [PATCH 1/2] cp,ln,mv: improve dir vs nondir diagnostics

* src/copy.c (copy_internal): Simplify logic for copying
from directory to non-directory or vice versa, and always
diagnose with both source and destination file names.
---
 src/copy.c | 88 --
 1 file changed, 26 insertions(+), 62 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index e7bf6022f..8b4a29692 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2451,72 +2451,36 @@ skip:
   if (return_now)
 return return_val;
 
-  if (!S_ISDIR (dst_sb.st_mode))
+  /* Copying a directory onto a non-directory, or vice versa,
+ is ok only with --backup.  */
+  if (!S_ISDIR (src_mode) != !S_ISDIR (dst_sb.st_mode)
+  && x->backup_type == no_backups)
 {
-  if (S_ISDIR (src_mode))
-{
-  if (x->move_mode && x->backup_type != no_backups)
-{
-  /* Moving a directory onto an existing
- non-directory is ok only with --backup.  */
-}
-  else
-{
-  error (0, 0,
-   _("cannot overwrite non-directory %s with directory %s"),
- quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
-  return false;
-}
-}
-
-  /* Don't let the user destroy their data, even if they try hard:
- This mv command must fail (likewise for cp):
-   rm -rf a b c; mkdir a b c; touch a/f b/f; mv a/f b/f c
- Otherwise, the contents of b/f would be lost.
- In the case of 'cp', b/f would be lost if the user simulated
- a move using cp and rm.
- Note that it works fine if you use --backup=numbered.  */
-  if (command_line_arg
-  && x->backup_type != numbered_backups
-  && seen_file (x->dest_info, dst_relname, _sb))
-{
-  error (0, 0,
- _("will not overwrite just-created %s with %s"),
- quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
-  return false;
-}
-}
-
-  if (!S_ISDIR (src_mode))
-{
-  if (S_ISDIR (dst_sb.st_mode))
-{
-  if (x->move_mode && x->backup_type != no_backups)
-{
-  /* Moving a non-directory onto an existing
- directory is ok only with --backup.  */
-}
-  else
-{
-  error (0, 0,
- _("cannot overwrite directory %s with non-directory"),
- quoteaf (dst_name));
-  return false;
-}
-}
+  error (0, 0,
+ _(S_ISDIR (src_mode)
+   ? ("cannot overwrite non-directory %s "
+  "with directory %s")
+   : ("cannot overwrite directory %s "
+  "with non-directory %s")),
+ quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
+  return false;

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/20/24 15:53, Bernhard Voelker wrote:


   $ echo 1 > a
   $ mkdir d
   $ echo 2 > d/a
   $ src/mv -v --exchange a a a d
   renamed 'a' -> 'd/a'
   renamed 'a' -> 'd/a'
   renamed 'a' -> 'd/a'
   $ cat a
   2
   $ src/mv -v --exchange a a a d
   renamed 'a' -> 'd/a'
   renamed 'a' -> 'd/a'
   renamed 'a' -> 'd/a'
   $ cat a
   1
   $ src/mv -v --exchange a a a a d
   renamed 'a' -> 'd/a'
   renamed 'a' -> 'd/a'
   renamed 'a' -> 'd/a'
   renamed 'a' -> 'd/a'
   $ cat a
   1


Yes, that's the expected behavior for this contrived case. Just as one 
would get odd behavior if one did the same thing without --exchange.




I remember some implementation where mv(1) really was just a rename(2),
which failed when crossing file systems.  Was it some HP-UX or Solaris mv(1)?


I doubt it. Even 7th Edition 'mv' (1979) fell back on 'cp' when the link 
syscall failed (this was before 'rename' existed).




My point is that "exchange" is a different functionality.


Yes, but it's closely related. Arguably --backup is also a different 
functionality too (and arguably --exchange is simply an alternative 
backup scheme!) but 'mv' has --backup.




- How large is the useful overlap with the existing code of mv(1)?
   Not much: no traditional rename nor copy.


I don't follow this point. The code change was fairly small, which 
indicates there was a lot of overlap with existing functionality.




- How large is the useful overlap with the existing options/modes of mv(1)?
   - exchange contradicts --backup,


That could be fixed for regular files, if there's a need, by backing up 
the destination via 'link' before exchanging. For directories it's 
admittedly a problem, but that's also the case for plain 'mv' (or for 
'cp' or 'ln', for that matter) so there's not much new here.




   - exchange is not useful together with options working with a regular
     rename of copy, at least: --update, -Z, -n.


It should work with --update and -Z. -n of course is logically 
incompatible, but this not the only set of logically incompatible 
options (e.g., -t vs -T).




   - not sure if exchange works well together with -f.


What problems do you see there?



why does exchange not work to exchange a regular with a 
directory file?


It works. I don't see a problem there.

  $ touch a
  $ mkdir d
  $ ./mv -T --exchange a d
  $ ls -ld a d
  drwxr-xr-x. 2 eggert eggert 4096 Mar 20 16:52 a
  -rw-r--r--. 1 eggert eggert0 Mar 20 16:52 d



Finally, the test cases are very sparse:


Feel free to add some. :-)

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/17/24 04:32, Pádraig Brady wrote:

I think the --no-copy situation is brittle, as scripts not using it now
would be atomic, but then if we ever supported cross fs swaps
it may become non atomic. I'd at least doc with a line in the --exchange
description in usage() to say something like:
   "Use --no-copy to enforce atomic operation"


But --no-copy doesn't mean atomic operation; it simply means don't copy.

On systems that don't have an atomic exchange, we can emulate "mv 
--exchange --no-copy a b" with three calls link("b", "b.tmp"); 
rename("a","b"); rename("b.tmp","a"). This wouldn't copy, but it 
wouldn't be atomic.


Although atomicity is important, currently the coreutils documentation 
doesn't cover it and the code doesn't always handle it well. For 
example, if A and B are regular files "mv -b A B" briefly has a moment 
when B stops existing. To my mind this is a bug: an existing destination 
shouldn't stop existing merely because you're replacing it. At some 
point this stuff should be documented better and this (and probably some 
other) atomicity bugs fixed.


One thought I had while looking into this was that we could add an 
--atomic option to mv and ln, such that the command fails if the 
destination cannot be updated atomically. That would be a stronger 
option than --no-copy. (In some cases we could support --atomic even for 
'cp', no?)


Anyway, for now I installed the patch with some minor changes to the 
documentation's --exchange section to try to document this tricky area 
more clearly. Here's the revised doc section. It also incorporates your 
later suggestion to mention both data and metadata.




@item --exchange
@opindex --exchange
Exchange source and destination instead of renaming source to destination.
Both files must exist; they need not be the same type.
This exchanges all data and metadata.

This option can be used to replace one directory with another.
When used this way, it should be combined with
@code{--no-target-directory} (@option{-T})
to avoid confusion about the destination location.
For example, you might use @samp{mv -T --exchange @var{d1} @var{d2}}
to exchange two directories @var{d1} and @var{d2}.

Exchanges are atomic if the source and destination are both in a
single file system that supports atomic exchange.
Non-atomic exchanges are not yet supported.

If the source and destination might not be on the same file system,
using @code{--no-copy} will prevent future versions of @command{mv}
from implementing the exchange by copying.

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/20/24 12:43, Bernhard Voelker wrote:


This stems from the fact that although mv(1) is a userland frontend
for renameat(2), the user interface is different:
while renameat(2) deals exactly with 2 operands, mv(1) has always
been able to work on more arguments.


Yes, that's mv's original sin, which we cannot realistically change now.



Now, extending "exchange" to more arguments is confusing and the
use is not intuitive:
   mv -v --exchange  a b c d

An "exchange" can literally only be applied to 2 files,


Sure, but that's true for "rename" too: a "rename" can be applied only 
to 2 files.


When d is a directory, "mv a b c d" does three renames so it is like "mv 
a d/a; mv b d/b; mv c d/c". This remains true if you uniformly replace 
"mv" with "mv --exchange", which does three exchanges.




I have the gut feeling that we didn't think through all cases,
and that some might be surprising, e.g.:

   $ mkdir d; echo 1 > a; echo 2 > d/a
   $ src/mv --exchange a a a a d/a

versus

   $ src/mv --exchange a a a a d/a


I don't understand the word "versus" here, as the two examples look the 
same to me.


If d/a is not a directory, the example is an error, just as it would be 
without --exchange.


If d/a is a directory and you have permissions etc., "mv a a a a d/a" is 
like attempting "mv -T a d/a/a; mv -T a d/a/a; mv -T a d/a/a; mv -T a 
d/a/a". If you use plain "mv" only the first "mv -T a d/a/a" succeeds 
because "a" goes away, so you get three diagnostics for the remaining 
three "a"s. If you use "mv --exchange" all four "mv --exchange -T a 
d/a/a" attempts succeed, and since there are an even number of exchanges 
the end result is a no-op except for updated directory timestamps. So I 
don't see any ambiguity about what mv should do with this contrived example.

bug#69901: (echo a; echo b) | sort -nu looses some data

[Paul Eggert]

On 3/19/24 08:33, Rafal Maszkowski wrote:

he --debug option advised in
README does not say anything helpful:

(echo a; echo b) | sort --debug -nu
sort: text ordering performed using simple byte comparison
a
^ no match for key


That diagnostic message is helpful. It's telling you that there's no 
number, so everything sorts as being the same, which means 'sort -nu' 
(correctly) outputs just one line.


'sort' has behaved this way for quite some time, and POSIX requires this 
behavior.

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 2024-03-05 06:16, Pádraig Brady wrote:
I think I'll remove the as yet unreleased mv --swap from coreutils, 
given that

util-linux is as widely available as coreutils on GNU/Linux platforms.


Although removing that "mv --swap" implementation was a win, I don't 
think we can simply delegate this to util-linux's exch command. 
Exchanging files via a renameat-like call is not limited to the Linux 
kernel; it's also possible on macOS via renameatx_np with RENAME_SWAP, 
and there have been noises about adding similar things to other 
operating systems.


I just now added support for macOS renameatx_np to Gnulib, so coreutils 
does not need to worry about the macOS details; it can simply use 
renameatu with the Linux flags. See:


https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=af32ee824ee18255839f9812b8ed61aa5257a82b

Even with Linux it's dicey. People may have older util-linux installed 
and so lack the 'exch' utility; this is true for both Fedora 39 and 
Ubuntu 23.10, the current releases. Ubuntu is also odd in that it 
doesn't install all the util-linux utilities as part of the util-linux 
package, so it's not clear what they will do with 'exch'.


So I propose that we implement the idea in coreutils in a better way, 
that interacts more nicely with -t, -T, etc. Also, I suggest using the 
Linuxish name "--exchange" instead of the macOSish name "--swap", and 
(for now at least) not giving the option a single-letter equivalent as I 
expect it to be useful from scripts, not interactively.


After looking at various ways to do it I came up with the attached 
proposed patch. This should work on both GNU/Linux and macOS, if your OS 
is recent enough and the file system supports atomic exchange.From d522aba06107d3532ad6103470727bf9057f8d2c Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 16 Mar 2024 22:50:17 -0700
Subject: [PATCH] mv: new option --exchange

* src/copy.h (struct cp_options): New member 'exchange'.
* src/copy.c (copy_internal): Support the new member.
* src/mv.c (EXCHANGE_OPTION): New constant.
(long_options): Add --exchange.
(usage): Document --exchange.
(main): Support --exchange.
* tests/mv/mv-exchange.sh: New test case.
* tests/local.mk (all_tests): Add it.
---
 NEWS|  7 ++
 doc/coreutils.texi  | 18 ++
 src/copy.c  | 54 +++--
 src/copy.h  |  4 +++
 src/mv.c| 16 +---
 tests/local.mk  |  1 +
 tests/mv/mv-exchange.sh | 41 +++
 7 files changed, 114 insertions(+), 27 deletions(-)
 create mode 100755 tests/mv/mv-exchange.sh

diff --git a/NEWS b/NEWS
index f21efc7c0..67bb27ebb 100644
--- a/NEWS
+++ b/NEWS
@@ -81,6 +81,13 @@ GNU coreutils NEWS-*- outline -*-
   and the command exits with failure status if existing files.
   The -n,--no-clobber option is best avoided due to platform differences.
 
+  mv now accepts an --exchange option, which causes the source and
+  destination to be exchanged.  It should be combined with
+  --no-target-directory (-T) if the destination is a directory.
+  The exchange is atomic if source and destination are on a single
+  file system that supports atomic exchange; --exchange is not yet
+  supported in other situations.
+
   od now supports printing IEEE half precision floating point with -t fH,
   or brain 16 bit floating point with -t fB, where supported by the compiler.
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index d07ed7e76..c456a03d9 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10269,6 +10269,24 @@ skip existing files but not fail.
 If a file cannot be renamed because the destination file system differs,
 fail with a diagnostic instead of copying and then removing the file.
 
+@item --exchange
+@opindex --exchange
+Exchange source and destination instead of renaming source to destination.
+Both files must exist; they need not be the same type.
+The exchange is atomic if the source and destination are both in a
+single file system that supports atomic exchange;
+exchanges are not yet supported in other situations.
+
+This option can be used to replace one directory with another, atomically.
+When used this way, it should be combined with
+@code{--no-target-directory} (@option{-T})
+to avoid confusion about the destination location.
+Also, if the two directories might not be on the same file system,
+using @code{--no-copy} will prevent future
+versions of @command{mv} from implementing the exchange by copying.
+For example, you might use @samp{mv -T --exchange --no-copy
+@var{d1} @var{d2}} to exchange the directories @var{d1} and @var{d2}.
+
 @item -u
 @itemx --update
 @opindex -u
diff --git a/src/copy.c b/src/copy.c
index 8d99f8562..e7bf6022f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2223,9 +2223,11 @@ copy_internal (char const *src_name, char const *dst_name,
 {
   if (rename_er

bug#69770: [PATCH] build: strengthen 16 bit float support checks

[Paul Eggert]

On 2024-03-14 06:03, Pádraig Brady wrote:



How about leaving it AC_COMPILE_IFELSE, but ensuring that -O1 or better
is used when the compiler supports -O1? That way we don't have to worry
about running the program, because (with the "volatile") clang will
error out.

Alternatively perhaps there's some way to check for the bug using
preprocessor macros like __FLT16_MANT_DIG__, __FLT16_MAX_EXP__,
__clang_major__, and __aarch64__. (This could be more fragile, though,
as clang presumably will fix the bug eventually.)


That would probably work for this edge case, but it's brittle
and I'm worried about other combinations of
compiler versions, complier options, and architectures.


Sure, but one cannot resolve such worries completely, as compiler errors 
are inherently brittle: even a runtime test cannot prove their absence.


As I understand it, __bf16 is a real zoo: sometimes hardware supports 
it, sometimes it doesn't, sometimes the software emulation library is 
there, sometimes it's not, and so forth. Running a program on a 
development host is likely to not match the runtime behavior on a 
production host, so AC_RUN_IFELSE is reasonably likely to produce a 
false positive, which is worse than a false negative.


Another issue, which is somewhat related, is that coreutils's current 
macro BF16_SUPPORTED is too broad. Because __bf16 has so many bugs, 
currently it's probably a mistake to say __bf16 is fully supported 
anywhere. Even GCC is buggy; see for example:


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114347

which I just now discovered when writing this email. The question isn't 
so much whether __bf16 is supported, it's whether it's supported well 
enough to compile and run GNU od.



Come to think of it, for 'od' we might be better off avoiding __bf16 
entirely, and simply converting the bits by hand into a 'float' 
variable. This would likely be more portable, as it would work even with 
compilers that lack __bf16, or that have __bf16 bugs relevant to 'od'. 
And that way we could remove the configure-time test entirely. (There 
would be endianness issues but they're easily addressible.)

bug#69770: [PATCH] build: strengthen 16 bit float support checks

[Paul Eggert]

On 2024-03-12 19:24, Grisha Levit wrote:

- AC_COMPILE_IFELSE(
+ AC_RUN_IFELSE(


This sort of change would break cross-compilation, no?

How about leaving it AC_COMPILE_IFELSE, but ensuring that -O1 or better 
is used when the compiler supports -O1? That way we don't have to worry 
about running the program, because (with the "volatile") clang will 
error out.


Alternatively perhaps there's some way to check for the bug using 
preprocessor macros like __FLT16_MANT_DIG__, __FLT16_MAX_EXP__, 
__clang_major__, and __aarch64__. (This could be more fragile, though, 
as clang presumably will fix the bug eventually.)

bug#69535: update

[Paul Eggert]

On 2024-03-08 00:49, brian wrote:
Please consider this bug report to be closed. I'm not sure if/how I can 
do that via e-mail.


Thanks for following up, and good luck with your hardware or drivers. 
Closing the bug report.

bug#69636: Re: [PATCH] Improve quality of format-checking code for seq.

[Paul Eggert]

On 2024-03-08 05:47, Pádraig Brady wrote:


   seq $a | xargs printf -- 'x%.0s'

   printf "%${a}s" '' | tr ' ' x

   yes x | head -n$a | tr -d '\n'


Oh, I like those! Unfortunately the printf solution silently fails for 
me when a=100 due to a bug in Bash. Perhaps I'll get up the 
energy to propose a fix (at least it should *noisily* fail :-).

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/4/24 12:35, Pádraig Brady wrote:

Another point worth mentioning before changing this,
is that changing would make the --swap operation non symmetric.
I.e. `mv -x a d` would be different to `mv -x d a` where d in a directory.


Yes, of course. It's just like mv without the -x. After you've done an 
'mv a b', 'mv b a' will undo the rename, unless b is a directory.


I realize this business of the destination being a directory is a 
hassle. That's why I put in mv's -t and -T options years ago. But at 
this point it's such a well-known hassle that we really gotta stay 
consistent with it if we add options.


The "exch" shell command (in util-linux) is a different matter, as it 
has never had the target-directory hassle (i.e., it has always assumed -T).


In looking into this a bit more, I see a bunch of other gotchas. For 
example, 'mv -b' does not work with 'mv -x'. A bunch of other mv options 
also don't work with 'mv -x'. It's a real mess as-is and we can't ship 
it this way.

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/4/24 16:43, Dominique Martinet wrote:

Adding Rob to the loop because this impacts compatibility with
toybox/maybe busybox implementations


Busybox does not use RENAME_EXCHANGE, so this isn't a Busybox issue.

Toybox mv added -x to its development version yesterday:

https://github.com/landley/toybox/commit/a2419ad52d489bf1a84a9f3aa73afb351642c765

so there's little prior art there, and there's still plenty of time to 
fix its problems before exposing it to the world.




I also see --swap mostly used by scripts and this actually feels a bit
dangerous to me -- I'd *always* use this with -T.


Yes, it's a problem.

By "see --swap mostly used by scripts" I assume you mean scripts that 
haven't been written yet, assuming that nobody had -x until yesterday




(by the way, what's this "rename" command you speak of?


https://mirrors.edge.kernel.org/pub/linux/utils/util-linux/

Now that I've looked into it further, util-linux already has an "exch" 
command that does exactly what you want. This is the command that toybox 
should implement rather than try to simulate it with "mv -x" (which 
causes all sorts of problems).


That is, toybox should revert yesterday's change to "mv", and should 
implement "exch" instead.

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/4/24 15:37, Petr Malat wrote:

Why do you expect this?


I expect it because mv has always treated destination directories that 
way. This has been true since the 1970s. We should not change this basic 
mode of operation.




To fix this, 'mv -x' should respect the usual mv behavior with respect to
directories. For example, when D is a directory 'mv -x A B C D' should act
like 'mv A B C D' except that D's old entries should be renamed back to A B
and C. And the -t and -T options should work with -x the same way they work
when -x is not specified.


I do not think this is a good idea, because that operation is not
atomic.


There's nothing wrong with 'mv -x a b c d/' being nonatomic. "mv a b c 
d/" is not atomic either, and that's fine too.




Probably, the user wants to prepare a temporary directory
instead and then atomically swap it with the directory in use.


This usage was not at all obvious to me. If it's the intended use, I 
suggest inventing a new command, or adding -x to the 'rename' command 
instead. Pushing this flag onto 'mv', without making the flag reasonably 
orthogonal to mv's other options, would be a mistake because it would 
cause too much confusion.




Supporting swap for more than 2 files also brings a problem, when
the operation fails as then one doesn't know what has been swapped
and what not.


I don't see why not. The user can look at mv's diagnostics. It's the 
same as regular "mv a b c d/".

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

On 3/4/24 15:16, Petr Malat wrote:

  I prefer KISS principle and allowing
swapping just 2 paths.


In that case, the option should be added to the 'rename' command, not to 
'mv'.


It is not KISS to add an option to 'mv' that makes it act completely 
differently, such that most of mv's other options don't work or don't 
make sense. That would be complexity, not simplicity.


If you want simplicity, use the 'rename' command.

bug#69535: Problem with copying an EXTREMELY large file - cmp finds a mismatch

[Paul Eggert]

Try running 'strace -o tr cp data.dat original' and then look at the 
file 'tr' (which could be quite large). Look for the syscalls near the 
start, and near the end, of the bulk copy.


Quite possibly it's a bug in your Linux drivers or your firmware or 
hardware. For example, if you're using ZFS, see:


https://github.com/openzfs/zfs/issues/15526

The strace output might help figure this out.

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Paul Eggert]

Although I like the idea of exposing file swaps to the user, the first 
cut of 'mv -x' has significant problems.


I expect 'mv -x A B' to act like 'mv A B' except the destination must 
exist and is renamed back to A. However, this is not true for 'mv -x A 
B' when B is a directory; it renames B to A rather than renaming B/A to 
A as I expect. That is, 'mv -x' acts as if -T (--no-target-directory) is 
also specified. There is no way to get mv's traditional behavior, or to 
get mv -t behavior.


To fix this, 'mv -x' should respect the usual mv behavior with respect 
to directories. For example, when D is a directory 'mv -x A B C D' 
should act like 'mv A B C D' except that D's old entries should be 
renamed back to A B and C. And the -t and -T options should work with -x 
the same way they work when -x is not specified.


This needs to happen before the next coreutils release, to avoid 
confusion about 'mv -x'.

Re: sort dynamic linking overhead

[Paul Eggert]

Thanks for the patch. I was hoping that we didn't need to worry about 
older platforms needing -ldl. Oh well.


The patch causes 'configure' to search for dlopen even when there's no 
crypto library. 'configure' could instead use AC_SEARCH_LIBS only if the 
AC_LINK_IFELSE fails (or simply put AC_LINK_ELSE in an 'for LIB_DL in "" 
-ldl' loop). But perhaps it's better to leave things be, in case we ever 
need dlopen for something else.


Also, if I understand things correctly, with this patch it's 
theoretically possible to pass -ldl to gcc even when 'sort' doesn't do 
dynamic linking, and we could complicate configure.ac further to omit 
-ldl in that case. But I doubt whether it's worth worrying about this.

Re: sort dynamic linking overhead

[Paul Eggert]

On 2024-02-26 06:12, Pádraig Brady wrote:

On 26/02/2024 06:44, Yann Collet wrote:
  * xxhash128 is not a cryptographic hash function, so it doesn't 
attempt tobe random.


Just a correction : xxh128 does try to be random. And quite hardly: a 
significant amount of development is spent on ensuring this property.


It’s even tested with PractRand, and it could be used as a good random 
number generator.


Being non-cryptographic means that what it doesn’t try is to make sure 
no one can intentionally forge a hash collision from 2 different files 
(other than brute-forcing, which is impractical).


But that’s different, and I wouldn’t call this property “randomness”, 
even though randomness is a pre-requisite (but not sufficient in 
itself) to collision resistance.


Fair enough, I should have called it a different name since it's not 
really random. However, 'sort -R' does have problems when hashes have 
collisions, as it falls back on ordinary comparisons and thus ceases to 
be a "random" sort, so collision resistance is a good property to have 
if 'sort -R' is given adversarial input.




md5 shouldn't be considered as cryptographic anyway since it's broken.


Although MD5 is broken as a hash for a string, it's not clear to me that 
it's broken in the way that GNU 'sort -R' uses MD5. GNU 'sort -R' uses a 
random salt, does not report hashes to the user, and does not output 
anything until it's read all its input. It seems to me that breaking gnu 
'sort -R' would be harder than breaking HMAC-MD5, which itself is 
significantly harder than breaking MD5.


That being said, if MD5 is broken for GNU 'sort' then we should use a 
better hash.

Re: sort dynamic linking overhead

[Paul Eggert]

On 2023-10-09 06:48, Pádraig Brady wrote:


An incremental patch attached to use xxhash128 (0.8.2)
shows a good improvement (note avx2 being used on this cpu):


xxhash128 is not a cryptographic hash function, so it doesn't attempt to 
be random. Of course most people won't care - it's random "enough" - but 
it would be a functionality change.


blake2 is cryptographic and would be random, but would bloat the 'sort' 
executable with code that's hardly ever used.


To attack the problem in a more conservative way, I installed the 
attached patch into coreutils. With it, 'sort -R' continues to use MD5 
but on GNUish platforms 'sort' links libcrypto dynamically only if -R is 
used (Bruno's suggestion). This doesn't significantly affect 'sort -R' 
performance, and reduces the startup overhead of plain 'sort' to be what 
it was before we started passing -lcrypto to gcc by default (in 
coreutils 8.32).


I also toyed with changing MD5 to SHA512, but that hurt performance. For 
what it's worth, although I tested with an Intel Xeon W-1350, which 
supports SHA-NI as well as various AVX-512 options, I didn't see where 
libcrypto (at least on Ubuntu 23.10, which has OpenSSL 3.0.10) takes 
advantage of these special-purpose instructions.From 7f57ac2d20c144242953a8dc7d95b02df0244751 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sun, 25 Feb 2024 17:13:12 -0800
Subject: [PATCH] sort: dynamically link -lcrypto if -R

This saves time in the usual case, which does not need -lcrypto.
* configure.ac (DLOPEN_LIBCRYPTO): New macro.
* src/sort.c [DLOPEN_LIBCRYPTO && HAVE_OPENSSL_MD5]: New macros
MD5_Init, MD5_Update, MD5_Final.  Include "md5.h" after defining
them.  Include , and define new functions link_failure
and symbol_address.
(link_libcrypto): New function.
(random_md5_state_init): Call it before using crypto functions.
---
 configure.ac | 29 +
 src/sort.c   | 52 +++-
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index c7eca1b8d..043081b90 100644
--- a/configure.ac
+++ b/configure.ac
@@ -351,6 +351,35 @@ if test $utils_cv_localtime_cache = yes; then
   AC_DEFINE([LOCALTIME_CACHE], [1], [FIXME])
 fi
 
+# Should 'sort' link libcrypto dynamically?
+AS_CASE([$LIB_CRYPTO],
+  [-lcrypto],
+[# Check for dlopen and libcrypto dynamic linking in one program,
+ # as there's little point to checking them separately.
+ AC_CACHE_CHECK([for dlopen and whether libcrypto is linked dynamically],
+   [utils_cv_dlopen_libcrypto],
+   [utils_cv_dlopen_libcrypto=no
+saved_LIBS=$LIBS
+LIBS="$LIBS $LIB_CRYPTO"
+AC_LINK_IFELSE(
+  [AC_LANG_PROGRAM(
+ [[#include 
+   #include 
+ ]],
+ [[return !(dlopen ("libcrypto.so", RTLD_LAZY | RTLD_GLOBAL)
+&& SHA512 (0, 0, 0));]])],
+  [# readelf works with cross-builds; ldd works on more platforms.
+   AS_CASE([`(readelf -d conftest$EXEEXT || ldd conftest$EXEEXT
+ ) 2>/dev/null`],
+ [*libcrypto*],
+   [utils_cv_dlopen_libcrypto=yes])])
+LIBS=$saved_LIBS])
+ AS_CASE([$utils_cv_dlopen_libcrypto],
+   [yes],
+ [AC_DEFINE([DLOPEN_LIBCRYPTO], [1],
+[Define to 1 if dlopen exists and libcrypto is
+ linked dynamically.])])])
+
 # macOS >= 10.12
 AC_CHECK_FUNCS([fclonefileat])
 
diff --git a/src/sort.c b/src/sort.c
index dea7be45d..cefe381bf 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -39,7 +39,6 @@
 #include "hash.h"
 #include "heap.h"
 #include "ignore-value.h"
-#include "md5.h"
 #include "mbswidth.h"
 #include "nproc.h"
 #include "physmem.h"
@@ -2085,6 +2084,56 @@ getmonth (char const *month, char **ea)
   return 0;
 }
 
+/* When using the OpenSSL implementation, dynamically link only if -R.
+   This saves startup time in the usual (sans -R) case.  */
+
+#if DLOPEN_LIBCRYPTO && HAVE_OPENSSL_MD5
+/* In the typical case where md5.h does not #undef HAVE_OPENSSL_MD5,
+   trick md5.h into declaring and using pointers to functions not functions.
+   This causes the compiler's -lcrypto option to have no effect,
+   as sort.o no longer uses any crypto symbols statically.  */
+# define MD5_Init (*ptr_MD5_Init)
+# define MD5_Update (*ptr_MD5_Update)
+# define MD5_Final (*ptr_MD5_Final)
+#endif
+
+#include "md5.h"
+
+#if DLOPEN_LIBCRYPTO && HAVE_OPENSSL_MD5
+# include 
+
+/* Diagnose a dynamic linking failure.  */
+static void
+link_failure (void)
+{
+  error (SORT_FAILURE, 0, "%s", dlerror ());
+}
+
+/* Return a function pointer in HANDLE for SYMBOL.  */
+static void *
+symbol_address (void *handle, char const *symbol)
+{
+  void *address = dlsym (handle, symbol);
+  

bug#69261: Further discussion on option processing

[Paul Eggert]

On 2024-02-20 12:33, Mathias MICHEL via GNU coreutils Bug Reports wrote:

I don't understand why --hide is depending on whether FILEs were provided
or not to the command. This is the opposite of what Paul stated.


It's not the opposite of what I stated. I said that --hide affects only 
files that 'ls' finds in directories itself (e.g., via ls -R); it does 
not affect command-line arguments. That's the behavior you're observing, 
and that's the documented behavior.


As for "why", it's similar to ls's behavior with files starting with 
".". Normally ls doesn't display them, but if you give an explicit 
command-line argument (e.g., "ls -d .") it displays them, regardless of 
whether you've also specified -a or -A or whatever.


It's not likely that we'd change ls's behavior for the command-line 
arguments you gave, as it's been behaving this way for years and other 
people likely depend on this behavior. However, you can get get the 
behavior that you want by using a different set of command-line 
arguments (see my previous email), so you might try doing that.

bug#69261: 'ls' : --ignore does not apply on FILEs selection

[Paul Eggert]

On 2024-02-18 14:07, Mathias MICHEL via GNU coreutils Bug Reports wrote:


Is it expected that --ignore arg does not apply on globbed FILE ?


Yes. --ignore is about what 'ls' finds in directories, not about 
command-line arguments.



My goal is to avoid using grep or complex find args:

> ~/.ssh :: ls id_ed* | grep -v "\.pub$" > id_ed25519 id_ed25519.local 
id_ed25519_ori


Try this:

  ls --hide='[^i]*' --hide='i[^d]*' --hide='id[^_]*' --hide='id_[^e]*' 
--hide='id_e[^d]*' --hide='*.pub'


Admittedly ugly, but does the job.

bug#68871: Can't use od to print half-precision floats

[Paul Eggert]

Thanks. One minor comment about this:


+@item B
+brain 16 bit float
+@item H
+half precision float


It might be helpful explain these two formats, e.g., to cite:

https://en.wikipedia.org/wiki/Bfloat16_floating-point_format

and

https://en.wikipedia.org/wiki/Half-precision_floating-point_format

respectively, since the formats are new compared to the other formats 
being discussed.

bug#68871: Can't use od to print half-precision floats

[Paul Eggert]

On 2/1/24 13:59, Pádraig Brady wrote:


bfloat16 looks like a truncated single precision IEEE,
so we should be able to just pad the extra 16 bits with zeros
when converting to single precision internally for processing.


Sounds good. This would mean od could work even the platform doesn't 
support bfloat16_t, since od.c could fall back on the above code (though 
I suppose it could be endianness-dependent).

bug#68871: Can't use od to print half-precision floats

[Paul Eggert]

Oh, and another thought: suppose someone wants to use od on bfloat16_t 
values? They're popular in machine learning applications, and likely 
will be more popular than float16_t overall. See:


https://sourceware.org/pipermail/libc-alpha/2024-February/154382.html

bug#68871: Can't use od to print half-precision floats

[Paul Eggert]

Thanks for writing that. One suggestion I'd make is to change this:

 #ifndef FLT16_MAX
 /* This is just a place-holder to avoid a few '#if' directives.
In this case, the type isn't actually used.  */
 typedef float _Float16;
 #endif

to something like this:

 #ifdef FLT16_MAX
 typedef _Float16 float16;
 #else
 /* This is just a place-holder to avoid a few '#if' directives.
In this case, the type isn't actually used.  */
 typedef float float16;
 #endif

and use 'float16' thereafter. That way, the code doesn't usurp the 
system namespace on older platforms, some of which might define _Float16 
but not FLT16_MAX.

bug#62572: Bug#1058752: bug#62572: cp --no-clobber behavior has changed

[Paul Eggert]

On 1/31/24 06:06, Pádraig Brady wrote:

To my mind the most protective option takes precedence.


That's not how POSIX works with mv -i and mv -f. The last flag wins. I 
assume this is so that people can have aliases or shell scripts that 
make -i the default, but you can override by specifying -f on the 
command line. E.g., in mymv:


   #!/bin/sh
   mv -i "$@"

then "mymv -f a b" works as expected.

Wouldn't a similar argument apply to cp's --update options?

Or perhaps we should play it safe, and reject any combination of 
--update etc. options that are incompatible. We can always change our 
mind later and say that later options override earlier ones, or do 
something else that's less conservative.

bug#62572: Bug#1058752: bug#62572: cp --no-clobber behavior has changed

[Paul Eggert]

On 2024-01-30 03:18, Pádraig Brady wrote:

So we now have the proposed change as:

   - revert -n to old silent success behavior
   - document -n as deprecated
   - Leave --update=none as is (will be synonymous with -n)
   - Provide --update=none-fail to diagnose and exit failure


Thanks, that's a better proposal, but I still see several opportunities 
for confusion.


If I understand things correctly, cp --update=none is not synonymous 
with the proposed (i.e., old-behavior) cp -n, because -n overrides 
previous -i options but --update=none does not. Also, -n overrides 
either previous or following --update=UPDATE options, but --update=none 
overrides only previous --update=UPDATE options. (For what it's worth, 
FreeBSD -n overrides


Some of this complication seems to be for consistency with how mv 
behaves with -f, -i, -n, and --update, and similarly with how rm behaves 
with -f, -i, -I, and --interactive. To be honest I don't quite 
understand the reason for all this complexity, which suggests it should 
be documented somewhere (the manual?) if it isn't already.


This raises more questions:

* If we deprecate cp -n, what about mv -n? FreeBSD mv -n behaves like 
Coreutils mv -n: it silently does nothing and exits successfully. So 
there's no compatibility argument for changing mv -n's behavior. 
However, whatever --update option we add to cp (to output a diagnostic 
and exit with failure) should surely also be added to mv, to aid 
consistency.


* Should cp --update=none be changed so that it really behaves like the 
old cp -n, in that it overrides other options in ways that differ from 
how the other --update=UPDATE options behave? I'm leaning toward "no" as 
this adds complexity that I don't see the use for.


* If we don't change cp --update=none's overriding behavior, is it still 
OK to tell users to substitute --update=none for -n even though the two 
options are not exactly equivalent? I'm leaning towards "yes" but would 
like other opinions.

bug#62572: Bug#1058752: bug#62572: cp --no-clobber behavior has changed

[Paul Eggert]

On 1/29/24 08:11, Pádraig Brady wrote:


Right, that's why I'm still leaning towards my proposal in the last mail.


Well, I won't insist on doing nothing; however, the proposal needs 
ironing out and now's a good time to do it before installing changes.




   - revert to previous exit success -n behavior
   - document -n as deprecated
   - provide --update=noclobber to give exit failure functionality


So --update=noclobber would differ in meaning from the deprecated-in-9.5 
--no-clobber, but would agree in meaning with 9.4 --no-clobber? That 
sounds pretty confusing for future users. (And a nit: why should one 
spelling have a hyphen but the other doesn't?)



     - BTW, it probably makes sense to print a diagnostic for each 
skipped file here
   as it's exceptional behavior, for which we're exiting with 
failure for.


Coreutils 9.4 cp -n already does that, no? So I'm not sure what's being 
proposed here.


  $ touch a b
  $ cp -n a b; echo $?
  cp: not replacing 'b'
  1



   - the existing --update=none provides the exit success functionality


It seems to me that this proposal conflates two questions:

* What rules should cp use to decide whether to update a destination?

* When cp decides not to update a destination, what should it do? Exit 
with nonzero status? Output a diagnostic? Both? Neither?


Aren't these independent axes? If so, shouldn't they have independent 
options? For example, since we have --update=older, shouldn't there be a 
way to say "I want to copy A to B only if B is older than A, and I want 
the exit status to be zero only if A was copied to B"?

bug#62572: Bug#1058752: bug#62572: cp --no-clobber behavior has changed

[Paul Eggert]

On 2024-01-28 05:22, Pádraig Brady wrote:

At this stage it seems best for us go back to the original Linux 
behiavor (use case 3),
and to silently deprecate -n in docs to document the portability issues 
with it.


I'm not sure reverting would be best. It would introduce more confusion, 
and would make coreutils incompatible with FreeBSD again.


The recent Debian change indicates that their intent is to move to the 
FreeBSD behavior too. This would improve cross-platform portability and 
I don't think we should discourage that.




  $ cp -n /bin/true tmp
  cp: warning: behavior of -n is non-portable and may change in future; use 
--update=none instead

This is problematic as:

  - It's noisy


Yes that's a problem, and I doubt whether we should mimic Debian.


  - There is no way to get the behavior of indicating failure if existing files 
present


Yes, it's not a good place to be. Surely current coreutils is better 
than what Debian is doing.



  - The --update=none advice is only portable to newer coreutils


True, but that's not a deal-killer. No advice that we give can be 100% 
portable to all platforms.



We should also provide --update=noclobber for use case 1.
Having the control on the --update option, allows use to more clearly 
deprecate -n.


Adding an --update=noclobber sounds like a good thing to do.

Another possibility is to add a warning that is emitted only at the end 
of 'cp'. The warning would occur only if the exit code differs because 
of this cp -n business. We could stretch things a bit and have a 
configure-time option --enable-compat-warnings that builders like Debian 
could use if they want such warnings.

bug#68267: [PATCH] maint: add attributes to two functions without side effects

[Paul Eggert]

On 2024-01-06 07:34, Pádraig Brady wrote:

Though I'm not seeing this suggestion with GCC 13.2.1,
so perhaps GCC 12 can determine the loops are finite?

I'll apply this since GCC 13 is less that a year old,
but in general we try to avoid littering code like this.


I would not apply this, as it's a bug in GCC's warning code that has 
been fixed. There is no need to apply these attributes to static 
functions - as I understand it, with current GCC, either GCC figures out 
the attributes anyway (so no need for humans to do it) or it doesn't (so 
it doesn't know to warn). The attributes can help efficiency a bit with 
small extern functions, but with small static functions they're not 
worth the trouble.


More generally, we don't need to cater to older compilers simply to 
suppress false-positive warnings. We can simply tell users that they can 
either ignore the harmless warnings or upgrade to the current GCC.


Although there are some exceptions to this guideline (e.g., .h files 
that are intended to be used by other people) this doesn't seem to be 
one of them.

bug#62572: cp --no-clobber behavior has changed

[Paul Eggert]

On 2023-12-16 13:46, Bernhard Voelker wrote:

Whether the implementation is race-prone or not is an internal thing.


I wasn't referring to the internal implementation. I was referring to cp 
users. With the newer Coreutils (FreeBSD) behavior, you can reliably 
write a script to do something if cp -n didn't copy the file because the 
destination already existed. With the older Coreutils behavior you 
cannot do that reliably; there will always be a race condition.

bug#62572: cp --no-clobber behavior has changed

[Paul Eggert]

On 2023-12-15 10:49, Michael Stone wrote:

There's no compelling reason to force this change


Well, certainly nobody compelled us at gunpoint

Stlll, Pádraig gave a reasonable summary of why the change was made, 
despite its incompatibility with previous behavior. (One thing I'd add 
is that the FreeBSD behavior is inherently less race-prone.) It seemed 
like a good idea at the time all things considered, and to my mind still 
does.




Essentially the current situation is that -n shouldn't be used if you expect a 
certain behavior for this case and you are writing a script for linux systems. 
Maybe in 10 years you'll be able to assume the new behavior. Better to just 
tell people to not use it at all, and leave the historic behavior alone until 
everyone has stopped using -n entirely.


Even if we tell people not to use -n at all, that doesn't mean we should 
revert to the coreutils 9.1 behavior.


The cat is to some extent out of the bag. Unless one insists on (FreeBSD 
| coreutils 9.2-9.4), or insist on coreutils 7.1-9.1, one should not 
rely on cp -n failing or silently succeeding when the destination 
already exists. This will remain true regardless of whether coreutils 
reverts to its 7.1-9.1 behavior.

bug#13601: mv should not silently lose file extended attributes

[Paul Eggert]

On 12/11/23 12:03, Abraham S.A.H. via GNU coreutils Bug Reports wrote:

a sane default behaviour regarding extended attributes in mv and others?


What's wrong with the default behavior in current GNU mv? Please give a 
specific example (specify platform, filesystems, mv version, etc.).

bug#67593: `split --number=l/N` no longer splits evenly

[Paul Eggert]

That's not a bug, in that 'split' is behaving as documented. The first 
input line is one byte shorter than the second one. 'Split' divides the 
input into two regions, and because the first region happens to be one 
byte longer than the second region both input lines are sent to the 
first output file.


In older coreutils, 'split' used a different algorithm to compute region 
sizes, which worked better for your test case but considerably worse in 
others. For example, in older coreutils:


seq 50 >in
split -n l/71 in

created 43 files of size 0, 9 files of size 2, 18 files of size 3, and 
one file of size 69. Current coreutils splits much better: it creates 21 
files of size 0, 9 files of size 2, and 41 files of size 3.

bug#67490: [PATCH v2] tail: fix tailing sysfs files on large page kernels

[Paul Eggert]

On 11/30/23 12:11, Pádraig Brady wrote:
Though that will generally give 128K, which is good when processing all 
of a file,

but perhaps overkill when processing just the last part of a file.


The 128 KiB number was computed as being better for apps like 'sed' that 
typically read all or most of the file. 'tail' sometimes behaves that 
way (e.g., 'tail -c +10') and so 'tail' should use 128 KiB in those 
cases. The simplest way to do that is for 'tail' to use 128 KiB all the 
time - that would cost little for uses like plain 'tail' and it could be 
a significant win for uses like 'tail -c +10'.


(As an aside, the 128 KiB number was computed in 2014. These days 256 
KiB might be better if someone could take the time to measure)

bug#67490: [PATCH] tail: fix following /proc and /sys files when using a 64K page size

[Paul Eggert]

On 2023-11-27 08:24, dann frazier wrote:

+  if (fstat (fd, ) != 0)
+{
+  error (0, errno, _("cannot fstat %s"), quoteaf (pretty_filename));
+  return false;
+}
+
+  bufsize = ST_BLKSIZE (stats);



If fstat fails, that's no reason to exit. Just use bufsize = BUFSIZ and 
keep going. fstat can fail for reasons unrelated to what 'tail' needs 
(e.g., time_t overflow).

bug#66835: Heap buffer overread in expr in regexec.c in the check_arrival_add_next_nodes function.

[Paul Eggert]

Thanks. This is a bug in the glibc regular expression matcher. It's part 
of a well known series of bugs. See, for example:


https://sourceware.org/bugzilla/show_bug.cgi?id=12896
https://sourceware.org/bugzilla/show_bug.cgi?id=17356

It's not of much practical concern since the attacker should not have 
control of B in invocations like 'expr "$A" : "$B"'.

bug#66919: ls -l output is misaligned

[Paul Eggert]

Thanks for the quick fix for the bug I introduced.

better i18n for join, uniq, etc.

[Paul Eggert]

I installed the attached patches to GNU Coreutils so that join and uniq 
support multi-byte characters better out-of-the-box. This uses Gnulib's 
new mcel module which makes for simpler multi-byte processing than 
what's in Fedora's i18n patches for Coreutils. (I also hope it's faster, 
though I haven't tested this.)


The idea is to continue this process of using mcel for the other 
programs where vanilla Coreutils doesn't conform to POSIX in multi-byte 
locales.


The key patch is 0009. Patch 0010 brings in the Fedora tests for join 
and uniq in multi-byte locales; these tests pass for me.


Some work is still needed for ignoring case in join and uniq. As I 
understand it, the Fedora patches don't support 'uniq --ignore-case' in 
multi-byte locales. They do support 'join --ignore-case', though they 
ignore it in the simple-minded way that GNU diff does (except diff 
lowercases first whereas Fedora join uppercases first; although neither 
approach is perfect isn't lowercasing better?).


Comments welcome. If the idea isn't a good one we can back out the 
patches. But I hope this can move forward.From 0292a5678a19cb3f3908cf3b267aa1f18b479aac Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 27 Oct 2023 08:45:50 -0700
Subject: [PATCH 01/11] maint: prefer c_isxdigit when that is the intent

* src/digest.c (valid_digits, split_3):
* src/echo.c (main):
* src/printf.c (print_esc):
* src/ptx.c (unescape_string):
* src/stat.c (print_it):
When the code is supposed to support only POSIX-locale hex digits,
use c_isxdigit rather than isxdigit.  Include c-ctype.h as needed.
This defends against oddball locales where isxdigit != c_isxdigit.
---
 src/digest.c | 5 +++--
 src/echo.c   | 5 +++--
 src/printf.c | 5 +++--
 src/ptx.c| 3 ++-
 src/stat.c   | 5 +++--
 5 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/digest.c b/src/digest.c
index b996dde11..1f3695308 100644
--- a/src/digest.c
+++ b/src/digest.c
@@ -23,6 +23,7 @@
 
 #include "system.h"
 #include "argmatch.h"
+#include "c-ctype.h"
 #include "quote.h"
 #include "xdectoint.h"
 #include "xstrtol.h"
@@ -660,7 +661,7 @@ valid_digits (unsigned char const *s, size_t len)
 {
   for (idx_t i = 0; i < digest_hex_bytes; i++)
 {
-  if (!isxdigit (*s))
+  if (!c_isxdigit (*s))
 return false;
   ++s;
 }
@@ -856,7 +857,7 @@ split_3 (char *s, size_t s_len,
 # endif
   unsigned char const *hp = *digest;
   digest_hex_bytes = 0;
-  while (isxdigit (*hp++))
+  while (c_isxdigit (*hp++))
 digest_hex_bytes++;
   if (digest_hex_bytes < 2 || digest_hex_bytes % 2
   || BLAKE2B_MAX_LEN * 2 < digest_hex_bytes)
diff --git a/src/echo.c b/src/echo.c
index 278778ec6..f80ead86f 100644
--- a/src/echo.c
+++ b/src/echo.c
@@ -19,6 +19,7 @@
 #include 
 #include "system.h"
 #include "assure.h"
+#include "c-ctype.h"
 
 /* The official name of this program (e.g., no 'g' prefix).  */
 #define PROGRAM_NAME "echo"
@@ -219,12 +220,12 @@ just_echo:
 case 'x':
   {
 unsigned char ch = *s;
-if (! isxdigit (ch))
+if (! c_isxdigit (ch))
   goto not_an_escape;
 s++;
 c = hextobin (ch);
 ch = *s;
-if (isxdigit (ch))
+if (c_isxdigit (ch))
   {
 s++;
 c = c * 16 + hextobin (ch);
diff --git a/src/printf.c b/src/printf.c
index f36b45519..ebe09ba76 100644
--- a/src/printf.c
+++ b/src/printf.c
@@ -56,6 +56,7 @@
 #include 
 
 #include "system.h"
+#include "c-ctype.h"
 #include "cl-strtod.h"
 #include "quote.h"
 #include "unicodeio.h"
@@ -262,7 +263,7 @@ print_esc (char const *escstart, bool octal_0)
 {
   /* A hexadecimal \xhh escape sequence must have 1 or 2 hex. digits.  */
   for (esc_length = 0, ++p;
-   esc_length < 2 && isxdigit (to_uchar (*p));
+   esc_length < 2 && c_isxdigit (to_uchar (*p));
++esc_length, ++p)
 esc_value = esc_value * 16 + hextobin (*p);
   if (esc_length == 0)
@@ -292,7 +293,7 @@ print_esc (char const *escstart, bool octal_0)
esc_length > 0;
--esc_length, ++p)
 {
-  if (! isxdigit (to_uchar (*p)))
+  if (! c_isxdigit (to_uchar (*p)))
 error (EXIT_FAILURE, 0, _("missing hexadecimal number in escape"));
   uni_value = uni_value * 16 + hextobin (*p);
 }
diff --git a/src/ptx.c b/src/ptx.c
index 3601875ed..3cd84b2e9 100644
--- a/src/ptx.c
+++ b/src/ptx.c
@@ -24,6 +24,7 @@
 #include "system.h"
 #include 
 #include "argmatch.h"
+#include

bug#66698: I think hex decoding with basenc -d --base16 should be case-insensitive

[Paul Eggert]

On 10/25/23 06:27, Pádraig Brady wrote:

But thinking about it more we probably should allow lower case for base32.


Yes, I'm thinking the same.

While thinking (:-) I couldn't resist improving performance a bit by 
installing the attached. This also fixes an unlikely bug when isxdigit 
behavior is oddball.From f4a59d453ebfa6dcaf2dd1a89a64c1fa05af7a85 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 25 Oct 2023 08:45:15 -0700
Subject: [PATCH 1/3] build: update gnulib submodule to latest

---
 gnulib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnulib b/gnulib
index b8083ec2e..e0ae1a7f3 16
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit b8083ec2eeb22c20d299d75aae06e465cf9e4494
+Subproject commit e0ae1a7f324d6b9462735273bc5a2848c712f883
-- 
2.41.0

From dcc1514d9ac12a98bf3067c206ce8e756c604719 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 25 Oct 2023 14:43:32 -0700
Subject: [PATCH 2/3] basenc: tweak checks to use unsigned char

This tends to generate better code, at least on x86-64,
because callers are just as fast and callees can avoid a conversion.
* src/basenc.c: The following renamings also change the arg type
from char to unsigned char.  All uses changed.
(isubase): Rename from isbase.
(isubase64url): Rename from isbase64url.
(isubase32hex): Rename from isbase32hex.
(isubase16): Rename from isbase16.
(isuz85): Rename from isz85.
(isubase2): Rename from isbase2.

2023-10-24  Paul Eggert  

* src/basenc.c (struct base16_decode_context):
Simplify by storing -1 for missing nibbles.  All uses changed.
---
 src/basenc.c | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/basenc.c b/src/basenc.c
index b47bd013f..850dd4cd9 100644
--- a/src/basenc.c
+++ b/src/basenc.c
@@ -216,7 +216,7 @@ static_assert (DEC_BLOCKSIZE % 40 == 0); /* Complete encoded blocks are used. */
 # define base_decode_context base32_decode_context
 # define base_decode_ctx_init base32_decode_ctx_init
 # define base_decode_ctx base32_decode_ctx
-# define isbase isbase32
+# define isubase isubase32
 #elif BASE_TYPE == 64
 # define BASE_LENGTH BASE64_LENGTH
 # define REQUIRED_PADDING base64_required_padding
@@ -232,7 +232,7 @@ static_assert (DEC_BLOCKSIZE % 12 == 0); /* Complete encoded blocks are used. */
 # define base_decode_context base64_decode_context
 # define base_decode_ctx_init base64_decode_ctx_init
 # define base_decode_ctx base64_decode_ctx
-# define isbase isbase64
+# define isubase isubase64
 #elif BASE_TYPE == 42
 
 
@@ -247,7 +247,7 @@ static_assert (DEC_BLOCKSIZE % 12 == 0); /* complete encoded blocks for base64*/
 
 static int (*base_length) (int i);
 static int (*required_padding) (int i);
-static bool (*isbase) (char ch);
+static bool (*isubase) (unsigned char ch);
 static void (*base_encode) (char const *restrict in, idx_t inlen,
 char *restrict out, idx_t outlen);
 
@@ -350,10 +350,10 @@ base64url_encode (char const *restrict in, idx_t inlen,
 }
 
 static bool
-isbase64url (char ch)
+isubase64url (unsigned char ch)
 {
   return (ch == '-' || ch == '_'
-  || (ch != '+' && ch != '/' && isbase64 (ch)));
+  || (ch != '+' && ch != '/' && isubase64 (ch)));
 }
 
 static void
@@ -463,7 +463,7 @@ static const char base32_hex_to_norm[32 + 9] = {
 
 
 inline static bool
-isbase32hex (char ch)
+isubase32hex (unsigned char ch)
 {
   return ('0' <= ch && ch <= '9') || ('A' <= ch && ch <= 'V');
 }
@@ -502,8 +502,8 @@ base32hex_decode_ctx_wrapper (struct base_decode_context *ctx,
   char *p = ctx->inbuf;
   while (i--)
 {
-  if (isbase32hex (*in))
-*p = base32_hex_to_norm[ (int)*in - 0x30];
+  if (isubase32hex (*in))
+*p = base32_hex_to_norm[*in - 0x30];
   else
 *p = *in;
   ++p;
@@ -518,9 +518,9 @@ base32hex_decode_ctx_wrapper (struct base_decode_context *ctx,
 }
 
 static bool
-isbase16 (char ch)
+isubase16 (unsigned char ch)
 {
-  return isxdigit (to_uchar (ch));
+  return isxdigit (ch);
 }
 
 static int
@@ -614,7 +614,7 @@ z85_length (int len)
 }
 
 static bool
-isz85 (char ch)
+isuz85 (unsigned char ch)
 {
   return c_isalnum (ch) || strchr (".-:+=^!/*?&<>()[]{}@%$#", ch) != nullptr;
 }
@@ -796,7 +796,7 @@ z85_decode_ctx (struct base_decode_context *ctx,
 
 
 inline static bool
-isbase2 (char ch)
+isubase2 (unsigned char ch)
 {
   return ch == '0' || ch == '1';
 }
@@ -875,7 +875,7 @@ base2lsbf_decode_ctx (struct base_decode_context *ctx,
   continue;
 }
 
-  if (!isbase2 (*in))
+  if (!isubase2 (*in))
 return false;
 
   bool bit = (*in == '1');
@@ -919,7 +919,7 @@ base2msbf_decode_ctx (struct base_decode_context *ctx,
   continue;
 }
 
-  if (!isbase2 (*in))
+  if (!isubase2 (*in))
 return false;
 
   bool bit = (*in == '1');
@@ -1065,7

bug#66698: I think hex decoding with basenc -d --base16 should be case-insensitive

[Paul Eggert]

On 10/23/23 06:08, Pádraig Brady wrote:

However the default operation should be the
most common requirement (and also the RFC documented operation in this 
case).

A similar case I hit very frequently is pasting hex into bc, and it's
very annoying to have to convert to uppercase before doing this.


Doesn't the isbase16 function also need updating?

Also, shouldn't we do something similar for base 32, for consistency?

bug#66714: [FIXED] Expr substr on plus symbol

[Paul Eggert]

On 10/23/23 12:58, petabaud51 wrote:

Could you folks please add it to 'man expr'?


Man pages are supposed to be terse

bug#66294: Formal Bug Report in paste.c of Coreutils Version 9.0 by Klee

[Paul Eggert]

I looked at the text version of the bug report and don't understand it. 
Can you please rephrase the bug report so that you tell us how you 
invoked 'paste' (command line arguments and input file contents), what 
behavior you expected, and what behavior you observed? Please bear in 
mind that your readers might not know about klee. Thanks.

bug#66294: Fwd: bug#66294: Formal Bug Report in paste.c of Coreutils Version 9.0 by Klee

[Paul Eggert]

Forwarding this attachment to <66...@debbugs.gnu.org> as it got lost in 
my spam inbox when it got sent just to me.


 Forwarded Message 
Subject: Re: bug#66294: Formal Bug Report in paste.c of Coreutils 
Version 9.0 by Klee

Date: Mon, 02 Oct 2023 10:53:57 +
From: JediWisdomKeeper 
To: Paul Eggert 






Sent with Proton Mail secure email.

--- Original Message ---
On Monday, October 2nd, 2023 at 6:45 AM, Paul Eggert 
 wrote:




Would you please resend your bug report as plain text? Thanks.Command used: klee --simplify-sym-indices --write-cvcs --write-cov 
--output-module --max-memory=1000 --disable-inlining --optimize 
--use-forked-solver --use-cex-cache --libc=uclibc --posix-runtime 
--external-calls=all --only-output-states-covering-new 
--max-sym-array-size=4096 --max-solver-time=30s --max-time=1min --watchdog 
--max-memory-inhibit=false --max-static-fork-pct=1 --max-static-solve-pct=1 
--max-static-cpfork-pct=1 --switch-type=internal --search=random-path 
--search=nurs:covnew --use-batching-search --batch-instructions=1 
./paste.bc --sym-args 0 1 10 --sym-args 0 2 2 --sym-files 1 8 --sym-stdin 8 
--sym-stdout






  

Error1 output as reported by KLEE


ktest file : 'test01.ktest'
args   : ['./paste.bc', '--sym-args', '0', '1', '10', '--sym-args', 
'0', '2', '2', '--sym-files', '1', '8', '--sym-stdin', '8', '--sym-stdout']
num objects: 11
object  0: name: 'n_args'
object  0: size: 4
object  0: data: b'\x01\x00\x00\x00'
object  0: hex : 0x0100
object  0: int : 1
object  0: uint: 1
object  0: text: 
object  1: name: 'arg00'
object  1: size: 11
object  1: data: b'-\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff'
object  1: hex : 0x2d00ff
object  1: text: -..
object  2: name: 'n_args'
object  2: size: 4
object  2: data: b'\x02\x00\x00\x00'
object  2: hex : 0x0200
object  2: int : 2
object  2: uint: 2
object  2: text: 
object  3: name: 'arg01'
object  3: size: 3
object  3: data: b'\x00\x00\x00'
object  3: hex : 0x00
object  3: text: ...
object  4: name: 'arg02'
object  4: size: 3
object  4: data: b'\x00\x00\x00'
object  4: hex : 0x00
object  4: text: ...
object  5: name: 'A_data'
object  5: size: 8
object  5: data: b'\x00\x00\x00\x00\x00\x00\x00\x00'
object  5: hex : 0x
object  5: int : 0
object  5: uint: 0
object  5: text: 
object  6: name: 'A_data_stat'
object  6: size: 144
object  6: data: 
b'6\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\xff\xff\xff\xff\x01\x00\x00\x00\x00\x00\x00\x00\xa4\x81\x00\x00\xe8\x03\x00\x00\xe8\x03\x00\x00\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\x00\x10\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xc1\x93\x10e\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff{\xf0\x11e\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff{\xf0\x11e\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff'
object  6: hex : 
0x360001000100a481e803e8030010c19310657bf011657bf01165
object  6: text: 
6..e{..e{..e
object  7: name: 'stdin'
object  7: size: 8
object  7: data: b'\x00\x00\x00\x00\x00\x00\x00\x00'
object  7: hex : 0x
object  7: int : 0
object  7: uint: 0
object  7: text: 
object  8: name: 'stdin_stat'
object  8: size: 144
object  8: data: 
b'6\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\xff\xff\xff\xff\x01\x00\x00\x00\x00\x00\x00\x00\xa4\x81\x00\x00\xe8\x03\x00\x00\xe8\x03\x00\x00\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\x00\x10\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xc1\x93\x10e\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff{\xf0\x11e\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff{\xf0\x11e\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff'
object  8: hex : 
0x360001000100a481e803e8030010c19310657bf011657bf01165
object  8: text: 
6..e{..e{..e
object  9: name: 'stdout'
object  9: size: 1024
object  9: data: 
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x

bug#66294: Formal Bug Report in paste.c of Coreutils Version 9.0 by Klee

[Paul Eggert]

Would you please resend your bug report as plain text? Thanks.

bug#66253: sort manpage should be more explicit

[Paul Eggert]

On 9/28/23 04:22, Pádraig Brady wrote:


   -n, --numeric-sort  compare according to string numerical value.
     leading blanks, negative sign, decimal 
point,

     and thousands separators are supported.


Although a valiant effort this is likely to cause other trouble, as it 
uses multiple terms (blanks, decimal point, thousands separator) without 
explanation, and it omits the role of the locale. I suggest instead that 
we simply say "see the manual", and tighten up the manual to explain 
these and, while we're at it, other things (e.g., -0 vs 0).


I gave that a shot by installing the attached.

PS to Jorge: Changing behavior as you suggested would likely cause 
trouble, as many programs depend on the current behavior, which is 
standardized by POSIX here:


https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sort.html#tag_20_119_04From a2434d3e58e8ead6c4c92fd989da32fe648e1545 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 28 Sep 2023 18:02:25 -0700
Subject: [PATCH] sort: improve --help

Problem reported by Jorge Stolfi (bug#66253).
* src/sort.c (usage): Suggest looking at the manual for -n details.
---
 doc/coreutils.texi | 12 
 src/sort.c |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index ee3b1ce11..be4b610be 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -4678,18 +4678,22 @@ can change this.
 @opindex --numeric-sort
 @opindex --sort
 @cindex numeric sort
+@vindex LC_CTYPE
 @vindex LC_NUMERIC
 Sort numerically.  The number begins each line and consists
 of optional blanks, an optional @samp{-} sign, and zero or more
 digits possibly separated by thousands separators, optionally followed
 by a decimal-point character and zero or more digits.  An empty
-number is treated as @samp{0}.  The @env{LC_NUMERIC}
-locale specifies the decimal-point character and thousands separator.
-By default a blank is a space or a tab, but the @env{LC_CTYPE} locale
-can change this.
+number is treated as @samp{0}.  Signs on zeros and leading zeros do
+not affect ordering.
 
 Comparison is exact; there is no rounding error.
 
+The @env{LC_CTYPE} locale specifies which characters are blanks and
+the @env{LC_NUMERIC} locale specifies the thousands separator and
+decimal-point character.  In the C locale, spaces and tabs are blanks,
+there is no thousands separator, and @samp{.} is the decimal point.
+
 Neither a leading @samp{+} nor exponential notation is recognized.
 To compare such strings numerically, use the
 @option{--general-numeric-sort} (@option{-g}) option.
diff --git a/src/sort.c b/src/sort.c
index abee57d7a..5c86b8332 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -444,7 +444,8 @@ Ordering options:\n\
   -h, --human-numeric-sortcompare human readable numbers (e.g., 2K 1G)\n\
 "), stdout);
   fputs (_("\
-  -n, --numeric-sort  compare according to string numerical value\n\
+  -n, --numeric-sort  compare according to string numerical value;\n\
+see manual for which strings are supported\n\
   -R, --random-sort   shuffle, but group identical keys.  See shuf(1)\n\
   --random-source=FILEget random bytes from FILE\n\
   -r, --reverse   reverse the result of comparisons\n\
-- 
2.41.0

bug#66256: sorting NAN values with "general-numeric’

[Paul Eggert]

On my long list of things to do is to have sort -g sort more 
deterministically with NaNs. This could be done with the new totalorder 
and totalorderl functions in C23 Annex F.10.12.1, if available. The fix 
would not be portable (a these functions are newly sort-of-standardized 
and are often not available) but it should be better than nothing.


Of course the other problem is that there's no standard textual 
representation of NaN payloads (i.e., their fractions).

bug#65599: mv and cp give a pointless warning when moving/copying a directory

[Paul Eggert]

On 2023-09-02 04:18, Bruno Haible wrote:

chmod fails with EACCES, hence per POSIX
<https://pubs.opengroup.org/onlinepubs/9699919799/functions/chmod.html>
it's buggy.


Thanks, I reported the bug with chmod, chown, etc. to the linux-cifs 
mailing list, here:


https://lore.kernel.org/linux-cifs/fe8ab586-c697-583b-650d-3adac64df...@cs.ucla.edu/T/#u

I did some testing with fsetxattr and found that it has a similar issue 
even with ext4; for example, setxattr("/", "a", "b", 1, 0) fails with 
EACCES. This is not a POSIX violation, since POSIX lacks setxattr. 
However it's bad form. And it means coreutils needs the attached 
workaround not just for CIFS, but also for ext4 in some (rare) cases.


I installed the attached workaround on coreutils and am closing the bug 
report. Thanks again for mentioning it.From 9cd52bd9993163c2ef8b3d62b757c573fb5320df Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 2 Sep 2023 13:27:45 -0700
Subject: [PATCH] cp,mv,install: fix chmod on Linux CIFS

This bug occurs only when temporarily setting the mode to the
intersection of old and new modes when changing ownership.
* src/copy.c (owner_failure_ok): Treat EACCES like EPERM.
---
 NEWS   | 6 +++---
 src/copy.c | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 6801832f7..2adea1e11 100644
--- a/NEWS
+++ b/NEWS
@@ -4,9 +4,9 @@ GNU coreutils NEWS-*- outline -*-
 
 ** Bug fixes
 
-  cp, mv, install no longer issue spurious "failed to preserve ownership"
-  diagnostics when copying to GNU/Linux CIFS filesystems.  They do
-  this by working around a Linux CIFS bug.
+  cp, mv, and install no longer issue spurious diagnostics like "failed
+  to preserve ownership" when copying to GNU/Linux CIFS file systems.
+  They do this by working around some Linux CIFS bugs.
 
   numfmt options like --suffix no longer have an arbitrary 127-byte limit.
   [bug introduced with numfmt in coreutils-8.21]
diff --git a/src/copy.c b/src/copy.c
index 0b3de04f3..9f1f3e85a 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -3468,7 +3468,8 @@ chown_failure_ok (struct cp_options const *x)
 static bool
 owner_failure_ok (struct cp_options const *x)
 {
-  return ((errno == EPERM || errno == EINVAL) && !x->owner_privileges);
+  return ((errno == EPERM || errno == EINVAL || errno == EACCES)
+  && !x->owner_privileges);
 }
 
 /* Return the user's umask, caching the result.
-- 
2.39.2

bug#65599: mv and cp give a pointless warning when moving/copying a directory

[Paul Eggert]

On 2023-09-01 19:52, Paul Eggert wrote:
Yes please. If chmod also does not work coreutils might need more 
workarounds.


Just to be clear; please try it on a file where chmod should fail 
because you don't own the file and you're not root. chmod should fail 
with EPERM in this case; it it fails with EACCES it's buggy. Thanks.

bug#65599: mv and cp give a pointless warning when moving/copying a directory

[Paul Eggert]

On 2023-09-01 16:12, Bruno Haible wrote:

Well, you asked me to try it on / but / is an ext4 FS, not a CIFS FS
on my system. Should I try it also on a directory inside the CIFS mount?


Yes please. If chmod also does not work coreutils might need more 
workarounds.

bug#65599: mv and cp give a pointless warning when moving/copying a directory

[Paul Eggert]

Good, thanks for confirming that the chmod family does not have a 
similar bug.


I installed the attached workaround into coreutils master on Savannah. 
Please give it a try.From 5f971361608749e1245c5eb12096de2acd32bf83 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 1 Sep 2023 15:05:21 -0700
Subject: [PATCH] cp,mv,install: fix chown on Linux CIFS

* src/copy.c (chown_failure_ok): Also treat EACCES as OK.
---
 NEWS   |  4 
 src/copy.c | 11 +--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index c00ff0cf2..6801832f7 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,10 @@ GNU coreutils NEWS-*- outline -*-
 
 ** Bug fixes
 
+  cp, mv, install no longer issue spurious "failed to preserve ownership"
+  diagnostics when copying to GNU/Linux CIFS filesystems.  They do
+  this by working around a Linux CIFS bug.
+
   numfmt options like --suffix no longer have an arbitrary 127-byte limit.
   [bug introduced with numfmt in coreutils-8.21]
 
diff --git a/src/copy.c b/src/copy.c
index 485879525..0b3de04f3 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -3449,9 +3449,16 @@ chown_failure_ok (struct cp_options const *x)
 {
   /* If non-root uses -p, it's ok if we can't preserve ownership.
  But root probably wants to know, e.g. if NFS disallows it,
- or if the target system doesn't support file ownership.  */
+ or if the target system doesn't support file ownership.
 
-  return ((errno == EPERM || errno == EINVAL) && !x->chown_privileges);
+ Treat EACCES like EPERM and EINVAL to work around a bug in Linux
+ CIFS <https://bugs.gnu.org/65599>.  Although this means coreutils
+ will ignore EACCES errors that it should report, problems should
+ occur only when some other process is racing with coreutils and
+ coreutils is not immune to races anyway.  */
+
+  return ((errno == EPERM || errno == EINVAL || errno == EACCES)
+  && !x->chown_privileges);
 }
 
 /* Similarly, return true if it's OK for chmod and similar operations
-- 
2.39.2

bug#65599: mv and cp give a pointless warning when moving/copying a directory

[Paul Eggert]

On 2023-09-01 08:09, Bruno Haible wrote:

It applies to all 4 variants, as can be seen from applying it to a directory:

$ ./a.out /media/nas/bruno/dir1
lchown: Permission denied
fchownat: Permission denied
chown: Permission denied
fchown: Permission denied


Thanks, for coreutils it'd be helpful to now whether there's a similar 
problem with chmod-related syscalls. If you compile and run the attached 
program on a file that you don't own (e.g., './a.out /'), does it 
incorrectly issue "Permission denied" instead of "Operation not 
permitted" diagnostics?#define _GNU_SOURCE
#include 
#include 
#include 
#include 

int
main (int argc, char **argv)
{
  char const *file = argv[1] ? argv[1] : ".";
  struct stat st;
  if (lstat (file, ) < 0)
return perror ("lstat"), 1;
  mode_t mode = st.st_mode & 0;
  int status = 0;
  if (lchmod (file, mode) < 0)
perror ("lchmod"), status = 1;
  if (fchmodat (AT_FDCWD, file, mode, AT_SYMLINK_NOFOLLOW) < 0)
perror ("fchmodat"), status = 1;
  if (!S_ISLNK (st.st_mode))
{
  if (chmod (file, mode) < 0)
	perror ("chmod"), status = 1;
  int fd = openat (AT_FDCWD, file, O_RDONLY | O_NOFOLLOW);
  if (fd < 0)
	perror ("openat"), status = 1;
  else if (fchmod (fd, mode) < 0)
	perror ("fchmod"), status = 1;
}
  return status;
}

bug#65599: mv and cp give a pointless warning when moving/copying a directory

[Paul Eggert]

Thanks for the followup. I looked into fixing it in Gnulib and it 
appears that this would require an extra syscall (or maybe even two) per 
file when copying to a CIFS filesystem, which I'd rather avoid. So I'm 
leaning more towards working around the bug in coreutils. Before doing 
that, it'd be helpful to know whether the bug is limited to fchownat or 
(as I suspect) applies also to the other chown variants. Could you try 
running the attached program on your platform, to see whether the bug 
affects these other variants?


Also, could you let us know the kernel and glibc version? The bug should 
be documented in gnulib/doc, I'd think. (Plus, reported to the Linux 
and/or glibc maintainers)#define _GNU_SOURCE
#include 
#include 
#include 
#include 

int
main (int argc, char **argv)
{
  char const *file = argv[1] ? argv[1] : ".";
  struct stat st;
  if (lstat (file, ) < 0)
return perror ("lstat"), 1;
  int status = 0;
  if (lchown (file, st.st_uid, st.st_gid) < 0)
perror ("lchown"), status = 1;
  if (fchownat (AT_FDCWD, file, st.st_uid, st.st_gid, AT_SYMLINK_NOFOLLOW) < 0)
perror ("fchownat"), status = 1;
  if (!S_ISLNK (st.st_mode))
{
  if (chown (file, st.st_uid, st.st_gid) < 0)
	perror ("chown"), status = 1;
  int fd = openat (AT_FDCWD, file, O_RDONLY | O_NOFOLLOW);
  if (fd < 0)
	perror ("openat"), status = 1;
  else if (fchown (fd, st.st_uid, st.st_gid) < 0)
	perror ("fchown"), status = 1;
}
  return status;
}

bug#65659: RFC: changing printf(1) behavior on %b

[Paul Eggert]

On 2023-08-31 08:35, Eric Blake wrote:

Typing-wise, %#s as a synonym for %b is
probably going to be easier (less shell escaping needed).  Is there
any interest in a patch to coreutils or bash that would add such a
synonym, to make it easier to leave that functionality in place for
POSIX Issue 9 even when %b is repurposed to align with C2x?


Sounds good to me for coreutils.

bug#65599: mv and cp give a pointless warning when moving/copying a directory

[Paul Eggert]

On 2023-08-29 11:20, Bruno Haible wrote:

People say that "chown only works for root anyway" [1]


I don't think that is the issue here. fchownat is failing with EACCES, 
which is supposed to mean that search permission is denied on a 
component of the path prefix, but in Android I guess EACCES also means 
the calling process doesn't have the required permissions to change 
ownership (i.e., errno should be EPERM according to POSIX).


Clearly search permission is allowed, since there's a successful 
utimensat call with the same file name. So EACCES does not mean search 
permission is denied; it means something else.


Do fchown, chown, and lchown have similar bugs on Android?

Although the attached hacky coreutils patch (which I haven't installed) 
might solve the immediate problem, it sounds like this is something that 
Gnulib's fchownat (and maybe related modules) should fix on Android, so 
that the fix is everywhere and not just in mv+cp.


diff --git a/src/copy.c b/src/copy.c
index b9fff03a3..7b901ffc3 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -3460,6 +3460,12 @@ chown_failure_ok (struct cp_options const *x)
  But root probably wants to know, e.g. if NFS disallows it,
  or if the target system doesn't support file ownership.  */
 
+#ifdef __ANDROID__
+  /* Work around Android bug .  */
+  if (errno == EACCES && !x->chown_privileges)
+return true;
+#endif
+
   return ((errno == EPERM || errno == EINVAL) && !x->chown_privileges);
 }
 

bug#65617: coreutils 9.4: seg.fault in readutmp with systemd

[Paul Eggert]

Thanks for reporting that. I installed the attached patch into Gnulib 
and this should appear in the next coreutils release.From 1e6a26f9312bb47e070f94b17b14dc1a6ffbb74f Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 30 Aug 2023 18:26:52 -0700
Subject: [PATCH] readutmp: fix core dump if --enable-systemd
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Thorsten Kukuk <https://bugs.gnu.org/65617>.
* lib/readutmp.c (read_utmp_from_systemd):
Don’t assume session_ptr != NULL if num_sessions == 0.
In practice it can be null, and the man page OKs this behavior.
---
 ChangeLog  | 8 
 lib/readutmp.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index d4b2287307..9c9c89638c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2023-08-30  Paul Eggert  
+
+	readutmp: fix core dump if --enable-systemd
+	Problem reported by Thorsten Kukuk <https://bugs.gnu.org/65617>.
+	* lib/readutmp.c (read_utmp_from_systemd):
+	Don’t assume session_ptr != NULL if num_sessions == 0.
+	In practice it can be null, and the man page OKs this behavior.
+
 2023-08-30  Bruno Haible  
 
 	doc: Mention the module 'wchar-single'.
diff --git a/lib/readutmp.c b/lib/readutmp.c
index 0173b7e0c1..e99158677c 100644
--- a/lib/readutmp.c
+++ b/lib/readutmp.c
@@ -795,7 +795,7 @@ read_utmp_from_systemd (idx_t *n_entries, STRUCT_UTMP **utmp_buf, int options)
 {
   char **sessions;
   int num_sessions = sd_get_sessions ();
-  if (num_sessions >= 0)
+  if (num_sessions > 0)
 {
   char **session_ptr;
   for (session_ptr = sessions; *session_ptr != NULL; session_ptr++)
-- 
2.39.2

bug#65423: README-install contains invalid/outdated information about 32 bit time_t compared to NEWS in 9.3

[Paul Eggert]

On 8/22/23 02:58, Osipov, Michael (IN IT IN) wrote:

So no action is required anymore.


Thanks for confirming that. Closing the bug report.

bug#65423: README-install contains invalid/outdated information about 32 bit time_t compared to NEWS in 9.3

[Paul Eggert]

On 8/21/23 05:51, Osipov, Michael (IN IT IN) via GNU coreutils Bug 
Reports wrote:
in 9.3 year 2038 support with 64 bit time_t was made required [1], here 
on HP-UX this is a bit problematic, but that's not the actual problem. 
The diff between 9.2 and 9.3 [2] says how this can be fixed on platforms 
supporting both 32 and 64 bit, on HP-UX it'd be simply '+DD64', and how 
to avoid with "ac_year2038_required=no", but README-install on master 
[3] still contains '--disable-year2038' which obviously does not work. 
It simply needs to be updated to the content of [2].


Sorry, I don't understand. Why does --disable-year2038 not work on HP-UX 
for bleeding-edge coreutils on Savannah?


It sounds like you're suggesting that we revert the attached patch, but 
I don't see how that would be correct as bleeding-edge 
coreutils/configure no longer looks at ac_year2038_required.



I cannot compile everything in 64 bit mode since no one has written our old applications with portability in mind. It needs to remain 32 bit for now. 


I suggest compiling coreutils in 64-bit mode now. You can keep compiling 
your other applications in 32-bit mode. Even though HP-UX's end of life 
is the end of 2025, it's possible you'll run across a stray HP-UX file 
today with timestamp after 2038, and basic coreutils apps like 'ls' 
should work with such files.From ba128e628cfa0dd111cf235d965200d1cdf77f52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Mon, 8 May 2023 12:57:56 +0100
Subject: [PATCH] doc: adjust build instructions for disabling year 2038
 support

* README-install: Adjust the instructions as per recent gnulib updates.
---
 README-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README-install b/README-install
index 6ab5b4f8c..bbc034951 100644
--- a/README-install
+++ b/README-install
@@ -58,7 +58,7 @@ Although 32-bit builds fail if that forces time_t to be 32 bits, this
 can be fixed by using 64-bit builds.  For example, on AIX where GCC
 defaults to 32 bits, one can use "./configure CC='gcc -maix64' AR='ar
 -X64'"; similarly, on Solaris one can configure with CC='gcc -m64'.
-If all else fails one can configure with ac_year2038_required=no;
+If all else fails one can configure with --disable-year2038;
 however, this will mishandle timestamps after 2038, and please file
 bug reports for any such situations.
 
-- 
2.41.0

bug#65255: uptime's boot time is inconsistent after VM sleep & resume

[Paul Eggert]

On 2023-08-15 14:23, Bruno Haible wrote:

The other patch I mentioned, from
https://lists.gnu.org/archive/html/coreutils/2023-08/msg00028.html  ,
is also needed, for the "VM saved/sleep" change on NetBSD, OpenBSD, Minix,
as far as I understand.

Also, a typo in NEWS: s/Minux/Minix/


Thanks, I had missed that patch. I installed it and fixed the NEWS typo.

bug#65269: Possible null pointer dereference on the function cycle_check in rm

[Paul Eggert]

On 2023-08-14 00:05, Haoxin Tu wrote:

if the function `fts_read` get a return value of
NULL and the malloc from `fts->fts_cycle.state = malloc (sizeof
*fts->fts_cycle.state)` (Line 62 in fts_cycle.c) is NULL, the pointer
`fts->fts_cycle.state` will still keep 0 before the free operation `free
(sp->fts_cycle.state);` (Line 159 in fts_cycle.c), leading to free of
invalid address.


I don't see a problem, since 'free (0)' is valid and does nothing.

bug#65255: uptime's boot time is inconsistent after VM sleep & resume

[Paul Eggert]

Thanks for the further comments. I installed your patch, along with the 
attached additional patches. The first makes 'uptime' a bit more 
resilient in the case of utmp and other failures, and the second adds 
NEWS items as per your comments.From 1ea34cbf6a235f2436a3265ab9ded6f04748051e Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 15 Aug 2023 14:00:54 -0700
Subject: [PATCH 1/2] uptime: be more generous about read_utmp failure

* src/uptime.c (print_uptime): Check for overflow
when computing uptime.  Use C99-style decl after statements.
Do not let an idx_t value go negative.
(print_uptime, uptime): Be more generous about read_utmp failures,
or when read_utmp does not report the boot time.  Instead of
failing, warn but keep going, printing the information that we did
get, and then exit with nonzero status.
(print_uptime): Return the desired exit status.  Caller changed.
---
 src/uptime.c | 77 +++-
 1 file changed, 46 insertions(+), 31 deletions(-)

diff --git a/src/uptime.c b/src/uptime.c
index 813d29430..928829dfc 100644
--- a/src/uptime.c
+++ b/src/uptime.c
@@ -17,9 +17,11 @@
 /* Created by hacking who.c by Kaveh Ghazi gh...@caip.rutgers.edu.  */
 
 #include 
-#include 
 
+#include 
+#include 
 #include 
+
 #include "system.h"
 
 #if HAVE_SYSCTL && HAVE_SYS_SYSCTL_H && ! defined __GLIBC__
@@ -43,19 +45,11 @@
   proper_name ("David MacKenzie"), \
   proper_name ("Kaveh Ghazi")
 
-static void
-print_uptime (idx_t n, struct gl_utmp const *this)
+static int
+print_uptime (idx_t n, struct gl_utmp const *utmp_buf)
 {
-  idx_t entries = 0;
+  int status = EXIT_SUCCESS;
   time_t boot_time = 0;
-  time_t time_now;
-  time_t uptime;
-  intmax_t updays;
-  int uphours;
-  int upmins;
-  struct tm *tmn;
-  double avg[3];
-  int loads;
 
 #if HAVE_SYSCTL && ! defined __GLIBC__ \
 && defined CTL_KERN && defined KERN_BOOTTIME
@@ -81,35 +75,47 @@ print_uptime (idx_t n, struct gl_utmp const *this)
 
   /* Loop through all the utmp entries we just read and count up the valid
  ones, also in the process possibly gleaning boottime. */
-  while (n--)
+  idx_t entries = 0;
+  for (idx_t i = 0; i < n; i++)
 {
+  struct gl_utmp const *this = _buf[i];
   entries += IS_USER_PROCESS (this);
   if (UT_TYPE_BOOT_TIME (this))
 boot_time = this->ut_ts.tv_sec;
-  ++this;
 }
   /* The gnulib module 'readutmp' is supposed to provide a BOOT_TIME entry
  on all platforms.  */
   if (boot_time == 0)
-error (EXIT_FAILURE, errno, _("couldn't get boot time"));
-
-  time_now = time (nullptr);
-  uptime = time_now - boot_time;
-  updays = uptime / 86400;
-  uphours = uptime % 86400 / 3600;
-  upmins = uptime % 86400 % 3600 / 60;
-  tmn = localtime (_now);
+{
+  error (0, errno, _("couldn't get boot time"));
+  status = EXIT_FAILURE;
+}
+
+  time_t time_now = time (nullptr);
+  struct tm *tmn = time_now == (time_t) -1 ? nullptr : localtime (_now);
   /* procps' version of uptime also prints the seconds field, but
  previous versions of coreutils don't. */
   if (tmn)
 /* TRANSLATORS: This prints the current clock time. */
 fprintftime (stdout, _(" %H:%M:%S  "), tmn, 0, 0);
   else
-printf (_(" ??:  "));
-  if (uptime == (time_t) -1)
-printf (_("up  days ??:??,  "));
+{
+  printf (_(" ??:  "));
+  status = EXIT_FAILURE;
+}
+
+  intmax_t uptime;
+  if (time_now == (time_t) -1 || boot_time == 0
+  || ckd_sub (, time_now, boot_time) || uptime < 0)
+{
+  printf (_("up  days ??:??,  "));
+  status = EXIT_FAILURE;
+}
   else
 {
+  intmax_t updays = uptime / 86400;
+  int uphours = uptime % 86400 / 3600;
+  int upmins = uptime % 86400 % 3600 / 60;
   if (0 < updays)
 printf (ngettext ("up %"PRIdMAX" day %2d:%02d,  ",
   "up %"PRIdMAX" days %2d:%02d,  ",
@@ -118,10 +124,12 @@ print_uptime (idx_t n, struct gl_utmp const *this)
   else
 printf (_("up  %2d:%02d,  "), uphours, upmins);
 }
+
   printf (ngettext ("%td user", "%td users", select_plural (entries)),
   entries);
 
-  loads = getloadavg (avg, 3);
+  double avg[3];
+  int loads = getloadavg (avg, 3);
 
   if (loads == -1)
 putchar ('\n');
@@ -136,6 +144,8 @@ print_uptime (idx_t n, struct gl_utmp const *this)
   if (loads > 0)
 putchar ('\n');
 }
+
+  return status;
 }
 
 /* Display the system uptime and the number of users on the system,
@@ -147,12 +157,17 @@ uptime (char const *filename, int options)
 {
   idx_t n_users;
   struct gl_utmp *utmp_buf;
-  if (read_utmp (filename, _users, _buf, options) != 0)
-error (EXIT_FAILURE, errno, "%s", quotef (filename));
-
-  print_up

bug#65269: Possible null pointer dereference on the function cycle_check in rm

[Paul Eggert]

On 2023-08-13 02:32, Haoxin Tu wrote:


We have developed a new tool built on top of KLEE (http://klee.github.io/)
to
automatically test GNU Coreutils-9.0 and found there might be a possible
null pointer
dereference


Thanks, but this bug was fixed in coreutils 9.2 (2023-03-20), due to 
this patch:


https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=f17d397771164c1b0f77fea8fb0abdc99cf4a3e1

I suggest using your tool on the latest release, coreutils 9.3. Or even 
better you can test the coreutils bleeding edge, by executing these 
shell commands, as described in the README-hacking file:


  git clone https://git.savannah.gnu.org/git/coreutils.git
  cd coreutils
  ./bootstrap
  ./configure
  make

bug#65255: uptime's boot time is inconsistent after VM sleep & resume

[Paul Eggert]

On 2023-08-12 16:00, Bruno Haible wrote:


I see this as a bug. Find attached a fix for this bug. I'll provide a NEWS
entry afterwards, that summarizes the changes in coreutils + gnulib on the
various platforms.


Thanks, this looks good. A NEWS entry would be welcome.

Does this mean Gnulib's 'uptime' module is obsolete?

bug#64937: boot time on Linux

[Paul Eggert]

On 2023-08-09 19:14, Po Lu wrote:

This uses the uptime counter (which also results in an SELinux denial
for me, but different Android distributions have SELinux policies of
varying strictness), which cannot establish the precise time the system
started


Emacs doesn't need a precise boot time. All it really needs is an 
integer that uniquely identifies the current OS boot.



 since time elapses between the read from the uptime counter and
the read from the RTC.


Emacs allows for up to one second of slop in computing the boot time. 
(In other words, it assumes that reboots are at least one second apart.) 
So if there are minor errors in computing the boot time it should be OK. 
If the errors are greater than one second, though, lock-file may assume 
that locks are stale when they're not.

bug#64937: boot time on Linux

[Paul Eggert]

[For those cc'ed, the thread's at <https://bugs.gnu.org/64937#142>.]

On 2023-08-09 07:29, Bruno Haible wrote:


And on Alpine Linux, while /var/run/utmp is empty, its time stamp is
essentially the boot time.

The approach used by Emacs, namely to look at the time stamp of
/var/run/random-seed, is therefore essentially one of the best approaches.
It just needs to also look at /var/lib/systemd/random-seed and - on Alpine
Linux - /var/run/utmp .


Thanks for looking into this. Clearly Emacs had some catching up to do, 
since it was using a location for the random-seed file that current 
GNU/Linux distros no longer use. To try to fix this I installed the 
attached patch to Emacs master on Savannah.


This patch does not address the problem for Alpine, nor I suspect for 
Android. I suppose Alpine could use the timestamp of /var/run/utmp (or 
is that /run/utmp?) but I don't know how 'configure' would reliably 
detect it's being built or cross-built for Alpine. I'll cc this to 
Natanael Copa, who does the Alpine ports for Emacs, to see whether he 
can give advice.


Also, I don't know how Android records boot time so I'll cc this to Po 
Lu, the main developer for Emacs on Android.From cc0a30a876adffa5ec110df9f4e0f21097f6d73e Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 9 Aug 2023 12:06:25 -0700
Subject: [PATCH] Adjust to random-seed move

For some time, GNU/Linux systems have put their random-seed file
somewhere other than where src/filelock.c looks for it.
Catch up to this by having 'configure' scout for it.
* configure.ac (BOOT_TIME_FILE):
Define this at configure-time.
* nt/inc/ms-w32.h (BOOT_TIME_FILE): Override 'configure'.
* src/filelock.c (BOOT_TIME_FILE): Remove default definition,
since 'configure' defaults it now.
---
 configure.ac| 31 +++
 nt/inc/ms-w32.h |  1 +
 src/filelock.c  |  6 --
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6e080c1c666..56c8cf1ae05 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2625,6 +2625,37 @@ AC_DEFUN
 fi
 AC_SUBST([AUTO_DEPEND])
 
+AC_CACHE_CHECK([for old but post-boot file],
+  [emacs_cv_boot_time_file],
+  [AS_CASE([$opsys],
+ [*bsd|darwin|dragonfly],
+   [emacs_cv_boot_time_file='not needed'],
+ [emacs_cv_boot_time_file=unknown
+  AS_IF([test $cross_compiling = no],
+	[# systemd puts it in /var/lib/systemd.
+	 # initscripts puts it in /var/lib/urandom (previously /var/lib).
+	 # Linux drivers/char/random.c before 2022-02-21 suggests /var/run.
+	 for file in \
+	 /var/lib/systemd/random-seed \
+	 /var/lib/urandom/random-seed \
+	 /var/lib/random-seed \
+	 /var/run/random-seed
+	 do
+	   test -f $file && { emacs_cv_boot_time_file=$file; break; }
+	 done])])])
+AS_CASE([$emacs_cv_boot_time_file],
+  [/*|*:*], [BOOT_TIME_FILE=\"$emacs_cv_boot_time_file\"],
+  [NULL|nullptr|0], [BOOT_TIME_FILE=$emacs_cv_boot_time_file],
+  ['not needed'], [BOOT_TIME_FILE=NULL],
+  [# Guess systemd if unknown.
+   # If guess is wrong, Emacs falls back on something else.
+   BOOT_TIME_FILE=\"/var/lib/systemd/random-seed\"])
+AC_DEFINE_UNQUOTED([BOOT_TIME_FILE], [$BOOT_TIME_FILE],
+  [Name of file that, if it exists, postdates boot and predates
+   the first Emacs invocation; or a null pointer if no such file is known.
+   This file is used only on GNU/Linux and other systems
+   that lack the FreeBSD-style sysctl with KERN_BOOTTIME.])
+
  Choose a window system.
 
 ## We leave window_system equal to none if
diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h
index 58be1199345..b23fd5030fa 100644
--- a/nt/inc/ms-w32.h
+++ b/nt/inc/ms-w32.h
@@ -121,6 +121,7 @@ #define HAVE_C99_STRTOLD 1
the output, but that's gross.  So this should do; if the file is
not there, the boot time will be returned as zero, and filelock.c
already handles that.  */
+#undef BOOT_TIME_FILE
 #define BOOT_TIME_FILE "C:/pagefile.sys"
 
 /*  */
diff --git a/src/filelock.c b/src/filelock.c
index 66b8fd2ceac..0ad130353f3 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -59,12 +59,6 @@ Copyright (C) 1985-1987, 1993-1994, 1996, 1998-2023 Free Software
 #include 
 #endif
 
-/* A file whose last-modified time is just after the most recent boot.
-   Define this to be NULL to disable checking for this file.  */
-#ifndef BOOT_TIME_FILE
-#define BOOT_TIME_FILE "/var/run/random-seed"
-#endif
-
 /* Boot time is not available on Android.  */
 
 #if defined HAVE_ANDROID && !defined ANDROID_STUBIFY
-- 
2.39.2

bug#64937: "who" reports funny dates

[Paul Eggert]

Thanks, I installed the attached to simplify a bit further. A nice 
consequence of these recent changes is that we get to remove some pragmas.


The first patch is for Gnulib; the other two are for Coreutils.From e14d7e198f96039dbc4fb2118739e6ca1fc4cec6 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 8 Aug 2023 19:29:55 -0700
Subject: [PATCH] readutmp: omit pragma
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/readutmp.c: Omit -Sstringop-overread pragma.
It’s no longer needed now that extract_trimmed_name
no longer calls strnlen.
---
 ChangeLog  | 7 +++
 lib/readutmp.c | 5 -
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 199c818c18..40274c0a08 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2023-08-08  Paul Eggert  
+
+	readutmp: omit pragma
+	* lib/readutmp.c: Omit -Sstringop-overread pragma.
+	It’s no longer needed now that extract_trimmed_name
+	no longer calls strnlen.
+
 2023-08-08  Bruno Haible  
 
 	readutmp: Use classical implementation for files != /var/run/utmp.
diff --git a/lib/readutmp.c b/lib/readutmp.c
index 66b25bc7ae..31db4023a1 100644
--- a/lib/readutmp.c
+++ b/lib/readutmp.c
@@ -136,11 +136,6 @@
 # pragma GCC diagnostic ignored "-Wsizeof-pointer-memaccess"
 #endif
 
-/* Work around <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110884>.  */
-#if 11 <= __GNUC__
-# pragma GCC diagnostic ignored "-Wstringop-overread"
-#endif
-
 /* Copy UT->ut_user into storage obtained from malloc.  Then remove any
trailing spaces from the copy, NUL terminate it, and return the copy.  */
 
-- 
2.39.2

From 422dcd424ca6b5fbef8d3bd0088e8e9e3757a203 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 8 Aug 2023 19:32:49 -0700
Subject: [PATCH 1/2] pinky,who: omit pragma

* src/pinky.c, src/who.c:
Omit no-longer-needed -Wstringop-overread pragma.
---
 src/pinky.c | 6 --
 src/who.c   | 6 --
 2 files changed, 12 deletions(-)

diff --git a/src/pinky.c b/src/pinky.c
index 1429dd073..59c552bf1 100644
--- a/src/pinky.c
+++ b/src/pinky.c
@@ -426,12 +426,6 @@ print_heading (void)
   putchar ('\n');
 }
 
-/* Work around <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109614>,
-   triggered by STREQ_LEN with a negative length.  */
-#if 11 <= __GNUC__
-# pragma GCC diagnostic ignored "-Wstringop-overread"
-#endif
-
 /* Display UTMP_BUF, which should have N entries. */
 
 static void
diff --git a/src/who.c b/src/who.c
index 27a7904e1..074d2d5ad 100644
--- a/src/who.c
+++ b/src/who.c
@@ -570,12 +570,6 @@ print_heading (void)
   _("PID"), _("COMMENT"), _("EXIT"));
 }
 
-/* Work around <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109614>,
-   triggered by STREQ_LEN with a negative length.  */
-#if 11 <= __GNUC__
-# pragma GCC diagnostic ignored "-Wstringop-overread"
-#endif
-
 /* Display UTMP_BUF, which should have N entries. */
 static void
 scan_entries (idx_t n, const STRUCT_UTMP *utmp_buf)
-- 
2.39.2

From ad823360267cbfb3f5b1b3e1438122e5c82eb0b2 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 8 Aug 2023 20:03:40 -0700
Subject: [PATCH 2/2] who: simplify based on readutmp changes

* src/pinky.c (time_string, print_entry, scan_entries, short_pinky):
* src/uptime.c (print_uptime, uptime):
* src/users.c (list_entries_users, users):
* src/who.c (UT_TYPE_RUN_LVL, UT_TYPE_INIT_PROCESS)
(UT_TYPE_LOGIN_PROCESS, UT_TYPE_DEAD_PROCESS, UT_TYPE_NEW_TIME)
(time_string, print_user, print_boottime)
(make_id_equals_comment, print_deadprocs, print_login)
(print_initspawn, print_clockchange, print_runlevel)
(list_entries_who, scan_entries, who):
Simplify, partly by using plain -> rather than macros.
---
 src/pinky.c  | 33 ++-
 src/uptime.c |  7 +++---
 src/users.c  |  5 ++---
 src/who.c| 63 ++--
 4 files changed, 44 insertions(+), 64 deletions(-)

diff --git a/src/pinky.c b/src/pinky.c
index 59c552bf1..5ad05e553 100644
--- a/src/pinky.c
+++ b/src/pinky.c
@@ -172,18 +172,10 @@ idle_string (time_t when)
 
 /* Return a time string.  */
 static char const *
-time_string (const STRUCT_UTMP *utmp_ent)
+time_string (struct gl_utmp const *utmp_ent)
 {
   static char buf[INT_STRLEN_BOUND (intmax_t) + sizeof "-%m-%d %H:%M"];
-
-  /* Don't take the address of UT_TIME_MEMBER directly.
- Ulrich Drepper wrote:
- "... GNU libc (and perhaps other libcs as well) have extended
- utmp file formats which do not use a simple time_t ut_time field.
- In glibc, ut_time is a macro which selects for backward compatibility
- the tv_sec member of a struct timeval value."  */
-  time_t t = UT_TIME_MEMBER (utmp_ent);
-  struct tm *tmp = localtime ();
+  struct tm *tmp = localtime (_ent->ut_ts.tv_sec);
 
   if (tmp)
 {
@@ -191,13 +183,13 @@ time_string (const STRUCT_UTMP *utmp_ent)
   return bu

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-08-08 05:24, Thorsten Kukuk wrote:


Something emacs needs to get fixed. On musl libc systems like Alpine,
you don't have utmp nor wtmp.


Yes, as Bruno mentioned, we know of no way to find the boot time on 
Alpine. When Emacs cannot determine the boot time, it pretends that the 
system booted in 1970.




Beside that the emacs heuristic to find backups of wmtp is very
questionable, it wouldn't match on any of my systems.


It should work on our department's old Solaris 10 system, which has 
files /var/adm/wtmpx, /var/adm/wtmpx.0, /var/adm/wtmpx.1, ..., 
/var/adm/wtmpx.9, each about a week older than its predecessor.




There are better ways to determine the boot time.


If we could find these better ways, we could add them to Emacs etc. In 
the meantime I'm not sure what we can do. As Bruno mentioned, 
CLOCK_BOOTTIME doesn't work either. This area is a bit of a mess.




Does they really maintain btmp or is it just openssh, where you cannot
disable btmp entries?


Ubuntu 23.04 really maintains btmp. I just now deliberately used the 
wrong password on a console, and an entry was created in btmp.




I know that Fedora tries to maintain it via pam_lastlog.so, but do to
all the problems with this interface that module is deprecated and will
be removed in a future release.


Perhaps they'll remove it in 2038. :-) In the meantime, we still need a 
substitute for utmp, wtmp, and btmp.

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-08-07 04:22, Bruno Haible wrote:


Since the fact that /var/run/utmp and /var/log/wtmp are world-readable
implies that they are world-lockable and thus the DoS bug
https://sourceware.org/bugzilla/show_bug.cgi?id=24492 applies,
to me it's clear that both utmp and wtmp needs to go away


Although that bug should be fixed, a fix doesn't require utmp and wtmp 
to go away. It can instead be fixed as Davin McCall suggested in the 
original bug report, by making the files not world-lockable. This should 
be doable on GNU/Linux (and I expect on other platform that support 
these files) by implementing his suggestion. Admittedly the fix has been 
low priority, but that doesn't mean it's not doable.




sooner than later. My guess is that Fedora and Ubuntu/Debian are only
waiting for 'who' (coreutils) and 'last' (util-linux / wtmpdb) to
stop accessing these two files.


It's not just those two programs. Emacs looks at utmp, for example, when 
creating the symlinks it uses to implement its own file locking, because 
 symlink contents contain the boot time (so that Emacs can better 
detect stale locks) and the boot time is retrieved from /var/run/utmp.


I expect that other programs look at utmp and/or wtmp, besides obvious 
candidates like 'login'. A quick scan through my Ubuntu /usr/bin found 
sessreg, for example; it was originally developed for X but is now used 
elsewhere.


Of course a better scheme could be arranged than utmp/wtmp. The problem 
is that it has not been arranged yet and progress in this area has been 
slow.




Although Ubuntu does not maintain /var/log/btmp


What do you mean by that?


Oh, my mistake. I checked a workstation that was behind a restrictive 
firewall, and nobody had ever attempted to attack it. You're right, 
Ubuntu maintains btmp.




   * The /var/log/wtmp argument to "who" and "users" should become deprecated.


If it is deprecated, we'll need a significant overlap period where the 
system both maintains utmp/wtmp/btmp, and also supports an alternative 
system. Ideally applications won't notice the transition, at least if 
they're using the POSIX/glibc interfaces in .




   * Whereas for /var/log/btmp we need to make an effort to continue supporting
 it, in the same 'who' program that accesses the systemd API for utmp.
 And access the time stamp in it as an unsigned 32-bit integer, like
 Andreas Schwab proposed (already implemented).


Although going to unsigned 32-bit integers is an improvement, it stops 
working in the year 2106 unless we install further hacks later. Better 
would be to widen the ?tmp files time_t to 64 bits, as is done on most 
other 64-bit platforms. This would require a flag-day conversion during 
a software upgrade, but that's doable, and -based programs 
should work after the upgrade is done.



 This means:
 (1) The API of the readutmp module should provide unlimited-length ut_user,
 ut_host etc. fields always. No more #ifdef UT_USER_SIZE.
 (2) The readutmp module should use a runtime 'if' rather than a 
compile-time
 #if, in order to dispatch between the systemd backend and the 
file-based
 backend.

 I'll work on (1) today.


Thanks. Both ideas sound good to me for Gnulib.

One issue with (1) is that readutmp.h currently defines a boatload of 
macros like UT_USER_SIZE, UT_TIME_MEMBER and UT_EXIT_E_TERMINATION that 
aren't needed if every platform simply uses struct gl_utmp. It'd be 
simpler to remove those macros (or move them to readutmp.c). Although it 
is also tempting to leave those macros in readutmp.h for backward 
compatibility, that might be more trouble than it's worth, as readutmp.h 
is already incompatible with how it was a week ago.

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-08-06 14:06, Thorsten Kukuk wrote:


SysV Init is gone and the majority does not miss it, and I don't see a
reason why "who /var/log/wtmp" needs to stay.


I don't want to get into the middle of another systemd vs init battle. 
But I don't see why this is relevant to that battle. Fedora 38 runs 
systemd, for example, and it still maintains /var/log/wtmp. Likewise for 
Ubuntu 23.04.




Is there somebody really using btmp? Beside that it is really unreliable
since nearly no application is writing it, I asked on several mailing
lists and nobody answered.


Although Ubuntu does not maintain /var/log/btmp, Fedora does. I just 
tested my Fedora 38 workstation, and /var/log/btmp records failed logins 
back when I installed Fedora on it. Attackers on the Internet try to log 
in to this workstation via ssh every few minutes. Logs like btmp could 
be useful for forensics if an attack succeeds.

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-08-06 14:10, Thorsten Kukuk wrote:

On Sun, Aug 06, Paul Eggert wrote:


On 2023-08-06 13:00, Paul Eggert wrote:


How does "last" emulate /var/log/wtmp using systemd?


Oh, I see from <https://github.com/thkukuk/wtmpdb> that wtmpdb comes
with its own "last". Is the plan to migrate this code into util-linux
"last", or simply to kill off util-linux "last"?  Similarly for
util-linux utmpdmp,  procps "w", etc.


util-linux will merge lastlog2, I have no idea about wtmpdb, this wasn't
a topic yet before my vacation.


Does lastlog2 store only the last login of each user? If so, it won't 
work for a command like "last eggert", which outputs all the times that 
I've logged in since wtmp was rotated.




procps-ng kills reading files directly and will use systemd, patch got
accepted but not yet merged.
The current patches for util-linux do the same.
I haven't looked at the latest PR for shadow, but they planned the same.

So nobody seems to care about "backward compatibily" with a time
where tools like "last" did not yet exist.


Hmm, well I guess I care. (Call me an old-timer. :-) Admittedly I care 
more about usage like "last eggert".




Also, the question about /var/adm/btmp remains.


Is there a use-case, for which it is __reliable__ useable?


I don't know what you mean by __reliable__. Certainly there are commands 
that use /var/adm/btmp. util-linux's "lastb" command uses it, for 
example. Will lastb stop working?




Until now, nobody could tell me one and nobody cared.


Perhaps you're not talking to the right people? I can find a reasonable 
amount of discussion about this stuff. For example, even if we restrict 
our attention to btmp, a Google search using the query "lastb btmp" 
returned 68,200 results for me just now, with many results on 
serverfault.com, stackexchange.com, linuxhint.com, etc.

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-08-06 13:00, Paul Eggert wrote:


How does "last" emulate /var/log/wtmp using systemd?


Oh, I see from <https://github.com/thkukuk/wtmpdb> that wtmpdb comes 
with its own "last". Is the plan to migrate this code into util-linux 
"last", or simply to kill off util-linux "last"?  Similarly for 
util-linux utmpdmp,  procps "w", etc.


Also, the question about /var/adm/btmp remains.

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-08-03 23:53, Thorsten Kukuk wrote:

And yes, "who /var/log/wtmp" will not work with this, too. But is this
really a required or usefull use case?
/usr/bin/last can give you the same output.


Sure, but some people use "who" for that.[1][2][3] This is partly 
because "who" predates "last".


How does "last" emulate /var/log/wtmp using systemd?

Also, what about /var/log/btmp? That is, what is the systemd substitute 
for "who /var/log/btmp" or "last -f /var/log/btmp"?


[1]: https://linuxhandbook.com/who-command/
[2]: https://linuxtldr.com/who-command/
[3]: https://kamotora.net/system/linux/check-btmp-wtmp-utmp/

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-08-04 04:58, Bruno Haible wrote:

The mentioned GCC bug is about a -Wanalyzer-use-after-free false positive.
I guess you meanthttps://gcc.gnu.org/bugzilla/show_bug.cgi?id=110884  or
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101778  ?


Thanks for catching that. Yes, it should be 110884. I installed the 
attached to Gnulib.From dcf286c586069684fe4d5bb2bd770394cd7cdad6 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sun, 6 Aug 2023 12:43:05 -0700
Subject: [PATCH] readutmp: fix comment bug ID

* lib/readutmp.c: Fix comment (thanks to Bruno Haible).
---
 lib/readutmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/readutmp.c b/lib/readutmp.c
index b8eba076fa..4e1d7ec26b 100644
--- a/lib/readutmp.c
+++ b/lib/readutmp.c
@@ -47,7 +47,7 @@
 # pragma GCC diagnostic ignored "-Wsizeof-pointer-memaccess"
 #endif
 
-/* Work around <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109614>.  */
+/* Work around <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110884>.  */
 #if 11 <= __GNUC__
 # pragma GCC diagnostic ignored "-Wstringop-overread"
 #endif
-- 
2.39.2

bug#64937: "who" reports funny dates

[Paul Eggert]

Thanks for doing all that work. I looked into it, and found a problem: a 
command like "who /var/log/wtmp" stops working because the systemd 
emulation of read_utmp only supports plain "who" (rougnly equivalent to 
"who /var/run/utmp" on Fedora). So I installed it into coreutils, but 
the default is systemd is disabled. This should give people time to 
experiment with it.


Thorsten, is there some way to get the equivalent of /var/log/wtmp with 
systemd?


Also, I simplified the use of the new readutmp interface a bit, by going 
back to the old way where you simply call 'free' once to free the 
storage. Although I toyed with the idea of simplifying readutmp.h 
further, by moving most of it into readutmp.c and having just struct 
gl_utmp public (this would simplify coreutils quite a bit), I ran out of 
time and patience and decided to ship what I had. So I nstalled the 
first ten attached patches into Gnulib, and the last patch into coreutils.


I haven't tested this with the latest systemd so there could well be 
typos in that part of the code.From 39a4cb0afdf4f2a1e6c2f3176b84e5b4cfe8996d Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 3 Aug 2023 15:31:48 -0700
Subject: [PATCH 1/9] readutmp: simplify extract_trimmed_name via ximemdup0

* lib/readutmp.c (extract_trimmed_name): Simplify.
* modules/readutmp (Depends-on):
Add strnlen, which was a missing dependency.

* lib/readutmp.c: Include xmemdup0.
(extract_trimmed_name): Simplify.
* modules/readutmp (Depends-on): Add xmemdup0.
Add strnlen, which was a missing dependency already.
---
 ChangeLog|  7 +++
 lib/readutmp.c   | 28 +++-
 modules/readutmp |  1 +
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b84cece6ff..7fa4e7b64a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2023-08-03  Paul Eggert  
+
+	readutmp: simplify extract_trimmed_name via ximemdup0
+	* lib/readutmp.c (extract_trimmed_name): Simplify.
+	* modules/readutmp (Depends-on):
+	Add strnlen, which was a missing dependency.
+
 2023-08-03  Bruno Haible  
 
 	alignasof, stdalign: Avoid some -Wundef warnings from config.h.
diff --git a/lib/readutmp.c b/lib/readutmp.c
index 11dd1655c5..9057a36494 100644
--- a/lib/readutmp.c
+++ b/lib/readutmp.c
@@ -47,29 +47,23 @@
 # pragma GCC diagnostic ignored "-Wsizeof-pointer-memaccess"
 #endif
 
+/* Work around <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109614>.  */
+#if 11 <= __GNUC__
+# pragma GCC diagnostic ignored "-Wstringop-overread"
+#endif
+
 /* Copy UT_USER (UT) into storage obtained from malloc.  Then remove any
trailing spaces from the copy, NUL terminate it, and return the copy.  */
 
 char *
 extract_trimmed_name (const STRUCT_UTMP *ut)
 {
-  char *p, *trimmed_name;
-
-#if READUTMP_USE_SYSTEMD
-  trimmed_name = xstrdup (UT_USER (ut));
-#else
-  trimmed_name = xmalloc (UT_USER_SIZE + 1);
-  strncpy (trimmed_name, UT_USER (ut), UT_USER_SIZE);
-  /* Append a trailing NUL.  Some systems pad names shorter than the
- maximum with spaces, others pad with NULs.  */
-  trimmed_name[UT_USER_SIZE] = '\0';
-#endif
-  /* Remove any trailing spaces.  */
-  for (p = trimmed_name + strlen (trimmed_name);
-   trimmed_name < p && p[-1] == ' ';
-   *--p = '\0')
-;
-  return trimmed_name;
+  char const *name = UT_USER (ut);
+  idx_t len = strnlen (name, UT_USER_SIZE);
+  char const *p;
+  for (p = name + len; name < p && p[-1] == ' '; p--)
+continue;
+  return ximemdup0 (name, p - name);
 }
 
 #if READ_UTMP_SUPPORTED
diff --git a/modules/readutmp b/modules/readutmp
index 04893a4487..484edd1842 100644
--- a/modules/readutmp
+++ b/modules/readutmp
@@ -12,6 +12,7 @@ extensions
 xalloc
 stdbool
 stdint
+strnlen
 sys_time
 fopen-gnu
 unlocked-io-internal
-- 
2.39.2

From 1ccde926759e8638d4b86de3dabbd948ad921edc Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 3 Aug 2023 15:53:27 -0700
Subject: [PATCH 2/9] =?UTF-8?q?readutmp:=20go=20back=20to=20simple=20?=
 =?UTF-8?q?=E2=80=98free=E2=80=99?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Omit the new free_utmp function.  Instead, allocate storage
in one block, so that using code can still just call ‘free’.
* lib/readutmp.c (struct utmp_alloc) [READUTMP_USE_SYSTEMD]: New type.
(add_utmp) [READUTMP_USE_SYSTEMD]: New function.
(read_utmp) [READUTMP_USE_SYSTEMD]: Use it.
Also, use malloc a bit less heavily.
(free_utmp): Remove.
* tests/test-readutmp.c (main): Call free, not free_utmp.
---
 ChangeLog |  10 ++
 NEWS  |   4 +-
 lib/readutmp.c| 285 +++---
 lib/readutmp.h|   4 -
 tests/test-readutmp.c |   4 +-
 5 files changed, 167 insertions(+), 140 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7fa4e7b64a..56b27d129f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2023-08-03  Paul 

bug#65003: pinky's "Idle" column lost the unit

[Paul Eggert]

Thanks for reporting that; I installed the attached.From 93e30466ff6eec8a2cd66374e199017763821478 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 2 Aug 2023 06:47:13 -0700
Subject: [PATCH] pinky: fix "d" typo

Problem reported by Bruno Haible (bug#65003).
* src/pinky.c (idle_string): Fix recently-introduced typo:
missing "d" for "days".
---
 src/pinky.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pinky.c b/src/pinky.c
index 381e753b6..b6d411c85 100644
--- a/src/pinky.c
+++ b/src/pinky.c
@@ -147,7 +147,7 @@ static char const *
 idle_string (time_t when)
 {
   static time_t now = 0;
-  static char buf[INT_STRLEN_BOUND (intmax_t) + 2];
+  static char buf[INT_STRLEN_BOUND (intmax_t) + sizeof "d"];
   time_t seconds_idle;
 
   if (now == 0)
@@ -165,7 +165,7 @@ idle_string (time_t when)
   else
 {
   intmax_t days = seconds_idle / (24 * 60 * 60);
-  sprintf (buf, "%"PRIdMAX, days);
+  sprintf (buf, "%"PRIdMAX"d", days);
 }
   return buf;
 }
-- 
2.39.2

Re: [PATCH] build: avoid use of C11 specific %t format size spec

[Paul Eggert]

On 2023-08-01 06:28, Pádraig Brady wrote:

perhaps we can now use %j, %t, and %z now?


I hope so, particularly as we haven't seen any issues with Coreutils 9.1 
or later.


I haven't run into problems using these printf formats for a while. 
Gnulib doc is ambiguous, but suggests it could be an issue on AIX 5.1, 
HP-UX 11.23, IRIX 6.5, Solaris 9, Cygwin 1.5.24, mingw, MSVC 14. Of 
these, the only platforms that are not obsolete are mingw and MSVC 14, 
and Stack Overflow[1] suggests that this is only with older MS-Windows 
versions and can be worked around easily on those older versions by 
whoever's doing the port.


Diffutils doesn't work on MS-Windows now anyway, as per Bug#64811, so 
I'll cc to that bug report to give Gisle Vanem a heads-up about this 
other issue.


[1]: 
https://stackoverflow.com/questions/44382862/how-to-printf-a-size-t-without-warning-in-mingw-w64-gcc-7-1

bug#64954: GNU 'uptime' on OpenBSD always prints "0 users"

[Paul Eggert]

Thanks, I propagated that into Coreutils and installed the simplified 
patch I mentioned yesterday. Closing the coreutils bug report.

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-07-31 00:08, Thorsten Kukuk wrote:

1. you need to extend ut_tv from 32bit time_t to 64bit time_t, or your
patch will not survive 2038.


Yes, that's the plan. Gnulib's readutmp module already does this, in 
apps that also use the year2038 or year2038-recommended modules (which 
most do).



2. the string entries are size limited in struct utmp, but are not with
libsystemd. So could be that you need to truncate device names, user
names and host names.


This suggests that Gnulib needs to redo the readutmp API so that it's 
not tied to struct utmpx's fixed string lengths. That's a bit of a 
hassle but should be doable (change char array to char *). An advantage 
is that Gnulib is a source-code library and so we can make binary- (and 
even source-) incompatible changes to the API without too much trouble.


Thanks for the advice.

bug#64954: GNU 'uptime' on OpenBSD always prints "0 users"

[Paul Eggert]

On 2023-07-30 11:41, Pádraig Brady wrote:

I'm fine with the change, but we'll also need to adjust
the sc_prohibit_always_true_header_tests syntax check in gnulib


I looked into that but it's such a hassle that I came up with the 
attached simpler patch to Coreutils. How about installing it instead? No 
Gnulib change should be needed.From 08cb549b6089519c1900339189398e66521b19ea Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sun, 30 Jul 2023 16:30:08 +0200
Subject: [PATCH] uptime: output correct user count on OpenBSD

* src/uptime.c (print_uptime, uptime): Always call read_utmp
and count the result.
* NEWS: Mention the fix (text by Bruno Haible).
---
 NEWS | 3 +++
 src/uptime.c | 7 ---
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index 92e591ee2..41205fa88 100644
--- a/NEWS
+++ b/NEWS
@@ -46,6 +46,9 @@ GNU coreutils NEWS-*- outline -*-
   'pr --length=1 --double-space' no longer enters an infinite loop.
   [This bug was present in "the beginning".]
 
+  'uptime' no longer incorrectly prints "0 users" on OpenBSD.
+  [bug introduced in coreutils-9.2]
+
   'wc -l' and 'cksum' no longer crash with an "Illegal instruction" error
   on x86 Linux kernels that disable XSAVE YMM.  This was seen on Xen VMs.
   [bug introduced in coreutils-9.0]
diff --git a/src/uptime.c b/src/uptime.c
index bc31026c0..b9d5e3c02 100644
--- a/src/uptime.c
+++ b/src/uptime.c
@@ -100,7 +100,6 @@ print_uptime (size_t n, const STRUCT_UTMP *this)
   }
 #endif
 
-#if HAVE_STRUCT_UTMP_UT_TYPE || HAVE_STRUCT_UTMPX_UT_TYPE
   /* Loop through all the utmp entries we just read and count up the valid
  ones, also in the process possibly gleaning boottime. */
   while (n--)
@@ -110,10 +109,6 @@ print_uptime (size_t n, const STRUCT_UTMP *this)
 boot_time = UT_TIME_MEMBER (this);
   ++this;
 }
-#else
-  (void) n;
-  (void) this;
-#endif
 
   time_now = time (nullptr);
 #if defined HAVE_PROC_UPTIME
@@ -177,10 +172,8 @@ uptime (char const *filename, int options)
   size_t n_users;
   STRUCT_UTMP *utmp_buf = nullptr;
 
-#if HAVE_STRUCT_UTMP_UT_TYPE || HAVE_STRUCT_UTMPX_UT_TYPE
   if (read_utmp (filename, _users, _buf, options) != 0)
 error (EXIT_FAILURE, errno, "%s", quotef (filename));
-#endif
 
   print_uptime (n_users, utmp_buf);
 
-- 
2.39.2

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-07-30 04:02, Pádraig Brady wrote:

Yes I think the consensus is to switch away from the utmp API,
which was recently discussed at:
https://lists.gnu.org/archive/html/coreutils/2023-06/msg00024.html


If I understand that discussion correctly, the idea is to switch from 
utmp/utmpx to the systemd interface once systemd 254 comes out.


As it happens, systemd 254 was published Friday. It'd be good to get it 
working with coreutils.


Thorsten's draft coreutils patches 
 are a bit 
long, and I'm hoping we can simplify this by packaging the fix inside 
Gnulib (much as we already packaged the fix for  not working on 
32-bit GNU/Linux x86 and ARM when _TIME_BITS is 64[1]), so that we 
needn't patch Coreutils other than to upgrade Gnulib version.


Is there any way to test this stuff easily on a platform like Fedora 38 
that is running systemd 253? That is, can I merely build the systemd 
libraries and link to them (how?), or would I have to replace the entire 
systemd ecosystem?


[1]: https://lists.gnu.org/r/bug-gnulib/2023-07/msg00159.html

bug#64937: "who" reports funny dates

[Paul Eggert]

On 2023-07-29 12:44, Pádraig Brady wrote:

I tried a quick build with -D__WORDSIZE_TIME64_COMPAT32=1
which is what glibc uses to force the smaller time types.
However that didn't fix the issue, so I'll need to look a bit more,
and how to get only utmp access restricted to 32 bit types.


I looked into that, and installed the attached patches into Gnulib and 
Coreutils respectively; these should work around the problem so I'll 
boldly close the bug report.


These patches are quite a hack, though, and (obviously) stop working 
after the year 2038.


What's Debian's and/or Fedora's plan for fixing /'s 
Y2038 bugs? (Or is the idea to remove the  API before 
2038? :-)


See:

https://lwn.net/Articles/925068/

https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#utmp_types_and_APIsFrom c408d9a53dbdaf48b555f216e250a2b3b8e48113 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 29 Jul 2023 17:06:42 -0700
Subject: [PATCH] readutmp: work around glibc utmpx bug

When compiled with _TIME_BITS=64, glibc  does not work,
because the files use 32-bit time_t and the code passes this to
the user unmodified, but  defines a struct with 64-bit
time_t.  Work around this compatibility bug.  Problem reported
by Jakub Wilk via Sven Joachim <https://bugs.gnu.org/64937>.
* lib/readutmp.c (copy_utmp_entry): New function.
(read_utmp): Use it.
---
 ChangeLog | 10 +++
 doc/glibc-functions/getutmp.texi  | 13 +
 doc/glibc-functions/getutmpx.texi | 13 +
 doc/glibc-headers/utmp.texi   | 13 +
 doc/posix-headers/utmpx.texi  | 13 +
 doc/year2038.texi |  8 ++
 lib/readutmp.c| 47 ++-
 7 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 53e22f3c98..aad4e95ef8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2023-07-29  Paul Eggert  
+
+	readutmp: work around glibc utmpx bug
+	When compiled with _TIME_BITS=64, glibc  does not work,
+	because the files use 32-bit time_t and the code passes this to
+	the user unmodified, but  defines a struct with 64-bit
+	time_t.  Work around this compatibility bug.
+	* lib/readutmp.c (copy_utmp_entry): New function.
+	(read_utmp): Use it.
+
 2023-07-29  Bruno Haible  
 
 	wcsrtombs tests: Renumber tests.
diff --git a/doc/glibc-functions/getutmp.texi b/doc/glibc-functions/getutmp.texi
index 2628441e13..b53369a597 100644
--- a/doc/glibc-functions/getutmp.texi
+++ b/doc/glibc-functions/getutmp.texi
@@ -26,4 +26,17 @@ Portability problems not fixed by Gnulib:
 @item
 This function is missing on some platforms:
 FreeBSD 13.0, OpenBSD 6.7, Minix 3.1.8, AIX 5.1, HP-UX 11, Cygwin 2.9, mingw, MSVC 14, Android 9.0.
+@item
+On some platforms, this function does not support timestamps past the
+year 2038:
+glibc 2.38 on 32-bit platforms like x86 and ARM where @code{time_t}
+was historically 32 bits.
+@item
+On some platforms, this function misbehaves if the @code{year2038} or
+@code{year2038-recommended} modules are used and the program is
+configured without the @option{--disable-year2038} option.
+The @code{readutmp} module works around this problem:
+glibc 2.38 on 32-bit platforms like x86 and ARM where @code{time_t}
+was historically 32 bits.
+@xref{Avoiding the year 2038 problem}.
 @end itemize
diff --git a/doc/glibc-functions/getutmpx.texi b/doc/glibc-functions/getutmpx.texi
index 0ebc01978b..bee96fc239 100644
--- a/doc/glibc-functions/getutmpx.texi
+++ b/doc/glibc-functions/getutmpx.texi
@@ -26,4 +26,17 @@ Portability problems not fixed by Gnulib:
 @item
 This function is missing on some platforms:
 FreeBSD 13.0, OpenBSD 6.7, Minix 3.1.8, AIX 5.1, HP-UX 11, Cygwin 2.9, mingw, MSVC 14, Android 9.0.
+@item
+On some platforms, this function does not support timestamps past the
+year 2038:
+glibc 2.38 on 32-bit platforms like x86 and ARM where @code{time_t}
+was historically 32 bits.
+@item
+On some platforms, this function misbehaves if the @code{year2038} or
+@code{year2038-recommended} modules are used and the program is
+configured without the @option{--disable-year2038} option.
+The @code{readutmp} module works around this problem:
+glibc 2.38 on 32-bit platforms like x86 and ARM where @code{time_t}
+was historically 32 bits.
+@xref{Avoiding the year 2038 problem}.
 @end itemize
diff --git a/doc/glibc-headers/utmp.texi b/doc/glibc-headers/utmp.texi
index d7d1091e14..1e9d3757cb 100644
--- a/doc/glibc-headers/utmp.texi
+++ b/doc/glibc-headers/utmp.texi
@@ -31,4 +31,17 @@ FreeBSD 8.0, OpenBSD 7.2.
 
 Portability problems not fixed by Gnulib:
 @itemize
+@item
+On some platforms, this API does not support timestamps past the
+year 2038:
+glibc 2.38 on 32-bit platforms like x86 and ARM where @code{time_t}
+was historically 32 bits.
+@item
+On some platforms, this header misbehaves if the @code{year2038} or
+@code{year2038-recommended} modules are used and the program is
+configured without the @option{--disable-year2038} 

bug#64785: Package: coreutils Version: 8.32-4+b1 program=mv

[Paul Eggert]

Thanks to both of you. I installed the attached into Savannah master 
coreutils. It implements the suggestion, except that later I noticed 
that EMLINK and ETXTBSY can join the throng.


At some point it might make sense to scan for other direct or indirect 
calls to renameat, renameat2, and linkat, where the diagnostic could be 
improved if it's known to refer to the destination.From 3cb862ce5f10db392cc8e6907dd9d888acfa2a30 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 22 Jul 2023 13:39:17 -0700
Subject: [PATCH] mv: better diagnostic for 'mv dir x' failure

Problem reported by Nir Oren <https://bugs.gnu.org/64785>.
* src/copy.c (copy_internal): Use a more-specific diagnostic when
a rename fails due to a problem that must be due to the
destination, avoiding user confusion in cases like 'mv dir x'
where x is a nonempty directory.
* tests/mv/dir2dir.sh: Adjust to match.
---
 NEWS|  7 +++
 src/copy.c  | 20 +---
 tests/mv/dir2dir.sh |  2 +-
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 7d2ca7f6b..bd0c59eb5 100644
--- a/NEWS
+++ b/NEWS
@@ -56,6 +56,13 @@ GNU coreutils NEWS-*- outline -*-
   short option is reserved to better support emulation of the standalone
   checksum utilities with cksum.
 
+  'mv dir x' now complains differently if x/dir is a nonempty directory.
+  Previously it said "mv: cannot move 'dir' to 'x/dir': Directory not empty",
+  where it was unclear whether 'dir' or 'x/dir' was the problem.
+  Now it says "mv: cannot overwrite 'x/dir': Directory not empty".
+  Similarly for other renames where the destination must be the problem.
+  [problem introduced in coreutils-6.0]
+
 ** Improvements
 
   cp, mv, and install now avoid copy_file_range on linux kernels before 5.3
diff --git a/src/copy.c b/src/copy.c
index ddc9eab30..90eebf6bc 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2840,9 +2840,23 @@ skip:
  If the permissions on the directory containing the source or
  destination file are made too restrictive, the rename will
  fail.  Etc.  */
-  error (0, rename_errno,
- _("cannot move %s to %s"),
- quoteaf_n (0, src_name), quoteaf_n (1, dst_name));
+  char const *quoted_dst_name = quoteaf_n (1, dst_name);
+  switch (rename_errno)
+{
+case EDQUOT: case EEXIST: case EISDIR: case EMLINK:
+case ENOSPC: case ENOTEMPTY: case ETXTBSY:
+  /* The destination must be the problem.  Don't mention
+ the source as that is more likely to confuse the user
+ than be helpful.  */
+  error (0, rename_errno, _("cannot overwrite %s"),
+ quoted_dst_name);
+  break;
+
+default:
+  error (0, rename_errno, _("cannot move %s to %s"),
+ quoteaf_n (0, src_name), quoted_dst_name);
+  break;
+}
   forget_created (src_sb.st_ino, src_sb.st_dev);
   return false;
 }
diff --git a/tests/mv/dir2dir.sh b/tests/mv/dir2dir.sh
index ea99eca56..da6a518e5 100755
--- a/tests/mv/dir2dir.sh
+++ b/tests/mv/dir2dir.sh
@@ -34,7 +34,7 @@ sed 's/: File exists/: Directory not empty/'o1;mv o1 out
 sed 's/: Device or resource busy/: Directory not empty/'o1;mv o1 out
 
 cat <<\EOF > exp || framework_failure_
-mv: cannot move 'b/t' to 'a/t': Directory not empty
+mv: cannot overwrite 'a/t': Directory not empty
 EOF
 
 compare exp out || fail=1
-- 
2.39.2

bug#64785: Package: coreutils Version: 8.32-4+b1 program=mv

[Paul Eggert]

On 2023-07-22 03:19, Pádraig Brady wrote:

Given the subtleties in this area,
I'd be reluctant to adjust diagnostics here.


I looked into this a bit more. Given "mv dir e" where e/dir is an 
existing nonempty directory, 7th Edition Unix fails this way:


  mv: e/dir exists

Solaris 10 mv fails this way:

  mv: cannot rename dir to e/dir: File exists

Coreutils 5.93 fails this way:

  mv: cannot overwrite directory `e/dir'

Current coreutils fails this way:

  mv: cannot move 'dir' to 'e/dir': Directory not empty

macOS fails this way:

  mv: rename dir to e/dir: Directory not empty

If you ask me, none of these are all that good. Current coreutils and 
macOS give a confusing "Directory not empty" message (which directory?). 
Solaris 10 has a similar confusion. 7th Edition and coreutils 5.93 don't 
print strerror (errno).


All things considered, how about if we go back to something like 
coreutils 5.93, except output strerror (errno) too? That is, something 
like this:


  mv: cannot overwrite 'e/dir': Directory not empty

This focuses the user on the problem, avoiding confusion from the 
irrelevant source file name. We'd use this format if renameat fails with 
an errno that means the problem must be with the destination, and stick 
with the current format otherwise. Affected errno values would be 
EDQUOT, EISDIR, ENOSPC, EEXIST, ENOTEMPTY.

bug#64540: [PATCH] od: exit out on failure to write to stdout

[Paul Eggert]

On 2023-07-17 10:12, Pádraig Brady wrote:

Right. In headers though, the traditional "static inline" idiom


Ah, I forgot that it was in system.h. At some point we should have 
system.h use _GL_INLINE but that can wait.

bug#64540: [PATCH] od: exit out on failure to write to stdout

[Paul Eggert]

On 2023-07-17 03:31, Pádraig Brady wrote:

static inline void


As a general rule, there's no need for 'static inline' in C, as nowadays 
compilers figure out inlining just fine for static functions and plain 
'static' should be good enough. There are exceptions but 'write_error' 
doesn't look like it's one of them.

bug#64604: readlink exit booelan value bug

[Paul Eggert]

On 2023-07-13 19:38, Budi wrote:

/tmp$ readlink -f ./KERNEL-linux-6.3.9  && echo SUCCEEDS FIND IT
/tmp/KERNEL-linux-6.3.9
SUCCEEDS FIND IT

but correct at deeper depth:

/tmp$ readlink -f ./KERNEL-linux-6.3.9/fs  && echo SUCCEEDS FIND IT



look it up, no KERNEL-linux-6.3.9 dir.:


That's not a bug. The documentation for -f says that the last component 
need not exist. Sounds like you want -e.