Bug#1037275: dd: regression - posix expression syntax no longer supported

2023-06-11 Thread Pádraig Brady

On 11/06/2023 18:29, Jim Meyering wrote:

Thanks. The patch looks fine.
My only suggestion would be a stylistic one, to change the ">"
comparison to be the equivalent "<" one, i.e., change this:

+  && *suffix == 'B' && (suffix > str && suffix[-1] != 'B'))

to this (also dropping the unnecessary parentheses):

+  && *suffix == 'B' && str < suffix && suffix[-1] != 'B')

Why? Because of the increasing-to-right number-line argument, where
smaller things are visually on the left of larger ones.

Currently, in src/, the uses of space-delimited < and <= outnumber
uses of > and >= by four to one, 1678 to 428.


Pushed with that adjustment.

thanks
Pádraig



Bug#1037275: dd: regression - posix expression syntax no longer supported

2023-06-11 Thread Jim Meyering
On Sat, Jun 10, 2023 at 6:50 AM Pádraig Brady  wrote:
> On 10/06/2023 02:45, Marc Lehmann wrote:
> > Package: coreutils
> > Version: 9.1-1
> > Severity: normal
> >
> > Dear Maintainer,
> >
> > I have a script that was used for some decades on multiple
> > unices. Beginning with bookworm, it stopped working because dd no longer
> > understands POSIX expression syntax for bs=:
> >
> > $ dd if=... bs=1024x1024x32
> > dd: invalid number: ‘1024x1024x32’
> >
> > This should be valid syntax according to POSIX, and was understood on
> > older versions of Debian GNU/Linux:
> >
> > Two or more positive decimal numbers (with or without k or b) separated 
> > by x, specifying the product of the indicated values
>
> Yes this was a regression in coreutils 9.1
> The patch attached is the proposed upstream fix.

Thanks. The patch looks fine.
My only suggestion would be a stylistic one, to change the ">"
comparison to be the equivalent "<" one, i.e., change this:

+  && *suffix == 'B' && (suffix > str && suffix[-1] != 'B'))

to this (also dropping the unnecessary parentheses):

+  && *suffix == 'B' && str < suffix && suffix[-1] != 'B')

Why? Because of the increasing-to-right number-line argument, where
smaller things are visually on the left of larger ones.

Currently, in src/, the uses of space-delimited < and <= outnumber
uses of > and >= by four to one, 1678 to 428.



Bug#1037275: dd: regression - posix expression syntax no longer supported

2023-06-10 Thread Pádraig Brady

On 10/06/2023 02:45, Marc Lehmann wrote:

Package: coreutils
Version: 9.1-1
Severity: normal

Dear Maintainer,

I have a script that was used for some decades on multiple
unices. Beginning with bookworm, it stopped working because dd no longer
understands POSIX expression syntax for bs=:

$ dd if=... bs=1024x1024x32
dd: invalid number: ‘1024x1024x32’

This should be valid syntax according to POSIX, and was understood on
older versions of Debian GNU/Linux:

Two or more positive decimal numbers (with or without k or b) separated by 
x, specifying the product of the indicated values


Yes this was a regression in coreutils 9.1
The patch attached is the proposed upstream fix.

thanks,
PádraigFrom 09186005a77bd24caedc7fa4fc3eb8acf44a4b50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sat, 10 Jun 2023 14:18:00 +0100
Subject: [PATCH] dd: fix parsing of numbers with more than two multipliers

* src/dd.c (parse_integer): Use recursion to support more than two
multipliers.  Also protect suffix[-1] access to ensure we don't
inspect before the passed string.
* tests/dd/bytes.sh: Add test cases.
* NEWS: Mention the bug fix.
Fixes https://bugs.debian.org/1037275
---
 NEWS  |  4 
 src/dd.c  |  8 
 tests/dd/bytes.sh | 12 
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index e47701962..7df9ff5b0 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,10 @@ GNU coreutils NEWS-*- outline -*-
   cksum again diagnoses read errors in its default CRC32 mode.
   [bug introduced in coreutils-9.0]
 
+  dd again supports more than two multipliers for numbers.
+  Previously numbers of the form '1024x1024x32' gave "invalid number" errors.
+  [bug introduced in coreutils-9.1]
+
   factor, numfmt, and tsort now diagnose read errors on the input.
   [This bug was present in "the beginning".]
 
diff --git a/src/dd.c b/src/dd.c
index 665fc831c..c826d09b8 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1428,7 +1428,7 @@ parse_integer (char const *str, strtol_error *invalid)
   intmax_t result;
 
   if ((e & ~LONGINT_OVERFLOW) == LONGINT_INVALID_SUFFIX_CHAR
-  && suffix[-1] != 'B' && *suffix == 'B')
+  && *suffix == 'B' && (suffix > str && suffix[-1] != 'B'))
 {
   suffix++;
   if (!*suffix)
@@ -1436,10 +1436,10 @@ parse_integer (char const *str, strtol_error *invalid)
 }
 
   if ((e & ~LONGINT_OVERFLOW) == LONGINT_INVALID_SUFFIX_CHAR
-  && *suffix == 'x' && ! (suffix[-1] == 'B' && strchr (suffix + 1, 'B')))
+  && *suffix == 'x')
 {
-  uintmax_t o;
-  strtol_error f = xstrtoumax (suffix + 1, , 10, , suffixes);
+  strtol_error f = LONGINT_OK;
+  intmax_t o = parse_integer (suffix + 1, );
   if ((f & ~LONGINT_OVERFLOW) != LONGINT_OK)
 {
   e = f;
diff --git a/tests/dd/bytes.sh b/tests/dd/bytes.sh
index 01753d6a0..21368ead6 100755
--- a/tests/dd/bytes.sh
+++ b/tests/dd/bytes.sh
@@ -60,4 +60,16 @@ for operands in "oseek=8B" "seek=8 oflag=seek_bytes"; do
   compare expected2 out2 || fail=1
 done
 
+# Check recursive integer parsing
+for oseek in '1x2x4 oflag=seek_bytes' '1Bx2x4' '1Bx8' '2Bx4B' '2x4B'; do
+  # seek bytes
+  echo abcdefghijklm |
+dd oseek=$oseek bs=5 > out 2> /dev/null || fail=1
+  compare expected out || fail=1
+done
+
+# Negative checks for integer parsing
+for count in B B1 Bx1 KBB BB KBb KBx x1 1xx1; do
+  returns_ 1 dd count=$count /dev/null || fail=1
+done
 Exit $fail
-- 
2.40.1



Bug#1037275: dd: regression - posix expression syntax no longer supported

2023-06-09 Thread Marc Lehmann
Package: coreutils
Version: 9.1-1
Severity: normal

Dear Maintainer,

I have a script that was used for some decades on multiple
unices. Beginning with bookworm, it stopped working because dd no longer
understands POSIX expression syntax for bs=:

   $ dd if=... bs=1024x1024x32
   dd: invalid number: ‘1024x1024x32’

This should be valid syntax according to POSIX, and was understood on
older versions of Debian GNU/Linux:

   Two or more positive decimal numbers (with or without k or b) separated by 
x, specifying the product of the indicated values

-- System Information:
Debian Release: 12.0
  APT prefers testing-security
  APT policy: (990, 'testing-security'), (990, 'testing'), (500, 
'unstable-debug'), (500, 'testing-debug'), (500, 'stable-debug'), (500, 
'unstable'), (1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386, x32

Kernel: Linux 6.1.32-schmorp (SMP w/24 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_USER, TAINT_OOT_MODULE, 
TAINT_UNSIGNED_MODULE
Locale: LANG=en_IE.UTF-8, LC_CTYPE=en_IE.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages coreutils depends on:
ii  libacl1  2.3.1-3
ii  libattr1 1:2.5.1-4
ii  libc62.36-9
ii  libgmp10 2:6.2.1+dfsg1-1.1
ii  libselinux1  3.4-1+b6

coreutils recommends no packages.

coreutils suggests no packages.

-- no debconf information