On 7/23/22 05:17, Pádraig Brady wrote:
BTW I see we've code in cache_fstatat() that assumes
st_size can't have such large values, which contradicts a bit.
Good catch. I installed the first attached patch.
> This is only a real consideration for virtual files I think
> since off_t is signed, and so impractical for a real file system
> to support files > OFF_T_MAX.
Yes, that sounds right.
You've convinced me that 'ls' should switch to the way 'stat' behaves
rather than vice versa; that's more useful anyway. How about the
attached second patch, which I haven't installed? (I was actually
inclined this way originally but got lazy.)From c2056a320b38126bf5566c2ce94e2c2b25243f66 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 23 Jul 2022 12:11:49 -0700
Subject: [PATCH 1/2] =?UTF-8?q?rm:=20don=E2=80=99t=20assume=20st=5Fsize=20?=
=?UTF-8?q?is=20nonnegative?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* src/remove.c: Include stat-time.h.
(cache_fstatat, cache_stat_init): Use negative st->st_atim.tv_sec to
determine whether the stat is cached, not negative st->st_size.
On non-POSIX platforms that lack st_atim.tv_sec, don’t bother to cache.
---
src/remove.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/remove.c b/src/remove.c
index b5d1ea8a2..e2f27ca4f 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -28,6 +28,7 @@
#include "ignore-value.h"
#include "remove.h"
#include "root-dev-ino.h"
+#include "stat-time.h"
#include "write-any-file.h"
#include "xfts.h"
#include "yesno.h"
@@ -62,29 +63,37 @@ enum Prompt_action
# define DT_LNK 2
#endif
-/* Like fstatat, but cache the result. If ST->st_size is -1, the
- status has not been gotten yet. If less than -1, fstatat failed
- with errno == ST->st_ino. Otherwise, the status has already
- been gotten, so return 0. */
+/* Like fstatat, but cache on POSIX-compatible systems. */
static int
cache_fstatat (int fd, char const *file, struct stat *st, int flag)
{
- if (st->st_size == -1 && fstatat (fd, file, st, flag) != 0)
+#if HAVE_STRUCT_STAT_ST_ATIM_TV_NSEC
+ /* If ST->st_atim.tv_nsec is -1, the status has not been gotten yet.
+ If less than -1, fstatat failed with errno == ST->st_ino.
+ Otherwise, the status has already been gotten, so return 0. */
+ if (0 <= st->st_atim.tv_nsec)
+ return 0;
+ if (st->st_atim.tv_nsec == -1)
{
- st->st_size = -2;
+ if (fstatat (fd, file, st, flag) == 0)
+ return 0;
+ st->st_atim.tv_nsec = -2;
st->st_ino = errno;
}
- if (0 <= st->st_size)
- return 0;
- errno = (int) st->st_ino;
+ errno = st->st_ino;
return -1;
+#else
+ return fstatat (fd, file, st, flag);
+#endif
}
/* Initialize a fstatat cache *ST. Return ST for convenience. */
static inline struct stat *
cache_stat_init (struct stat *st)
{
- st->st_size = -1;
+#if HAVE_STRUCT_STAT_ST_ATIM_TV_NSEC
+ st->st_atim.tv_nsec = -1;
+#endif
return st;
}
--
2.34.1
From 03cc716cb1d6d69dfdb9038a6889035ab957f201 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 23 Jul 2022 11:00:33 -0700
Subject: [PATCH 2/2] ls: print negative file sizes as negative
This is more useful in practice (Bug#56710).
However, if POSIXLY_CORRECT is set, print them as positive.
* src/ls.c (abs_file_size, human_file_size): New functions.
(gobble_file, print_long_format): Use them.
* src/stat.c: Revert previous change, so that stat and ls agree.
---
NEWS | 6 ++++--
src/ls.c | 53 +++++++++++++++++++++++++++++++++++++++++++----------
src/stat.c | 10 +---------
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/NEWS b/NEWS
index 816025255..d76946eb8 100644
--- a/NEWS
+++ b/NEWS
@@ -21,12 +21,14 @@ GNU coreutils NEWS -*- outline -*-
'cp --reflink=always A B' no longer leaves behind a newly created
empty file B merely because copy-on-write clones are not supported.
+ Unless POSIXLY_CORRECT is set, 'ls -l' no longer prints negative
+ file sizes as huge positive numbers. This is more consistent with
+ how 'stat -c %s' treats virtual files like /proc/kcore.
+
'ls -v' and 'sort -V' go back to sorting ".0" before ".A",
reverting to the behavior in coreutils-9.0 and earlier.
This behavior is now documented.
- ’stat -c %s' now prints sizes as unsigned, consistent with 'ls'.
-
** New Features
factor now accepts the --exponents (-h) option to print factors
diff --git a/src/ls.c b/src/ls.c
index d48892be7..475fb2719 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3142,6 +3142,19 @@ file_ignored (char const *name)
|| patterns_match (ignore_patterns, name));
}
+/* The following functions assumes typical implementations
+ where off_t is no wider than uintmax_t. */
+verify (OFF_T_MAX <= UINTMAX_MAX / 2);
+
+/* Return abs (SIZE) as an unsigned integer. */
+
+static uintmax_t
+abs_file_size (off_t size)
+{
+ uintmax_t s = size;
+ return size < 0 ? -s : s;
+}
+
/* POSIX requires that a file size be printed without a sign, even
when negative. Assume the typical case where negative sizes are
actually positive values that have wrapped around. */
@@ -3152,6 +3165,32 @@ unsigned_file_size (off_t size)
return size + (size < 0) * ((uintmax_t) OFF_T_MAX - OFF_T_MIN + 1);
}
+/* Store a string representation of the file size N into BUF,
+ which should have at LONGEST_HUMAN_READABLE + 2 bytes.
+ Return a pointer to the string. */
+
+static char *
+human_file_size (off_t n, char *buf)
+{
+ if (n < 0)
+ {
+ static signed char posixly;
+ if (posixly == 0)
+ posixly = ls_mode == LS_LS && getenv ("POSIXLY_CORRECT") ? 1 : -1;
+ if (posixly < 0)
+ {
+ char *p = human_readable (abs_file_size (n), buf + 1,
+ file_human_output_opts, 1,
+ file_output_block_size);
+ *--p = '-';
+ return p;
+ }
+ }
+
+ return human_readable (unsigned_file_size (n), buf,
+ file_human_output_opts, 1, file_output_block_size);
+}
+
#ifdef HAVE_CAP
/* Return true if NAME has a capability (see linux/capability.h) */
static bool
@@ -3623,12 +3662,8 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
}
else
{
- char buf[LONGEST_HUMAN_READABLE + 1];
- uintmax_t size = unsigned_file_size (f->stat.st_size);
- int len = mbswidth (human_readable (size, buf,
- file_human_output_opts,
- 1, file_output_block_size),
- 0);
+ char buf[LONGEST_HUMAN_READABLE + 2];
+ int len = mbswidth (human_file_size (f->stat.st_size, buf), 0);
if (file_size_width < len)
file_size_width = len;
}
@@ -4416,13 +4451,11 @@ print_long_format (const struct fileinfo *f)
}
else
{
- char hbuf[LONGEST_HUMAN_READABLE + 1];
+ char hbuf[LONGEST_HUMAN_READABLE + 2];
char const *size =
(! f->stat_ok
? "?"
- : human_readable (unsigned_file_size (f->stat.st_size),
- hbuf, file_human_output_opts, 1,
- file_output_block_size));
+ : human_file_size (f->stat.st_size, hbuf));
int pad;
for (pad = file_size_width - mbswidth (size, 0); 0 < pad; pad--)
*p++ = ' ';
diff --git a/src/stat.c b/src/stat.c
index 549762aba..3765a8f65 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -1492,14 +1492,6 @@ do_stat (char const *filename, char const *format,
}
#endif /* USE_STATX */
-/* POSIX requires 'ls' to print file sizes without a sign, even
- when negative. Be consistent with that. */
-
-static uintmax_t
-unsigned_file_size (off_t size)
-{
- return size + (size < 0) * ((uintmax_t) OFF_T_MAX - OFF_T_MIN + 1);
-}
/* Print stat info. Return zero upon success, nonzero upon failure. */
static bool
@@ -1583,7 +1575,7 @@ print_stat (char *pformat, size_t prefix_len, char mod, char m,
fail |= out_mount_point (filename, pformat, prefix_len, statbuf);
break;
case 's':
- out_uint (pformat, prefix_len, unsigned_file_size (statbuf->st_size));
+ out_int (pformat, prefix_len, statbuf->st_size);
break;
case 'r':
if (mod == 'H')
--
2.34.1
