Subject: bug#24874: dd: misleading parsing of hex numbers

From: Pádraig Brady <[email-masked]>
Date: Fri, 4 Nov 2016 17:03:29 +0000

On 04/11/16 16:20, Pádraig Brady wrote:
> On 04/11/16 11:19, Stephan Bauroth wrote:
>> Dear coreutils team :)
>>
>> I encountered a buglike behaviour of dd when handling skip and count 
>> parameters that are encoded in hex and thus prefixed with 0x.
>>
>> dd is not able to parse them, which is OK but would be great if it would 
>> be, but, worse, reads 0xf00 as 0. It does that silently. While an 
>> enduser will immediately notice this on count, since nothing is copied, 
>> behaviour for skip looks ok. (In fact, I noticed this only because I 
>> hexdumped the result after hours of debugging)
>>
>> While it's OK that dd can't parse these numbers, maybe there should be a 
>> warning that 0x was found and interpreted as 0. Since a char like 'x' is 
>> invalid within a number that by definition has to be decimal, a warning 
>> should be fairly easy to implement.
>>
>> Of course, the ability to parse hex numbers in these parameters would be 
>> awesome :)
>>
>> regards and thanks for your continuing work,
>> Stephan Bauroth
> 
> Ouch. That's a real gotcha.
> Note hex digits after the 0x are diagnosed, but not decimal digits:
> 
>   $ dd skip=0x100 seek=0xf00
>   dd: invalid number: ‘0xf00’
> 
> Disallowing 0x... could definitely break backwards compat though.
> Consider `for rec in 0 1 2; do dd skip=${rec}x1024...`
> 
> I suppose we could output a warning to suggest using
>   $(($rec * $size)) or 0${rec}x${size}
> if that really is the intention?
> 
> Given the warning workaround would be suggested in the message,
> and that it's a relatively rare usage, a warning is probably appropriate here.
> We already warn in dd for various usage.
> 
> I'll fix that for the coming release.

Patch attached.

>From 490edf8df09348eb2304392415f91ba4319840b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Fri, 4 Nov 2016 16:55:58 +0000
Subject: [PATCH] dd: warn about counts specified with confusing 0x prefix

* src/dd.c (parse_integer): Suggest to use "00x" instead of "0x",
which is significant for the "count", "seek", and "skip" operands.
* tests/dd/misc.sh: Add a test case.
Fixes http://bugs.gnu.org/24874
---
 src/dd.c         |  6 ++++++
 tests/dd/misc.sh | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/src/dd.c b/src/dd.c
index 2c6d4c6..c7f54d4 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1342,6 +1342,12 @@ parse_integer (const char *str, strtol_error *invalid)
           return 0;
         }
 
+      if (n == 0 && STRPREFIX (str, "0x"))
+        error (0, 0,
+               _("warning: %s is a zero multiplier; "
+                 "use %s if that is intended"),
+               quote_n (0, "0x"), quote_n (1, "00x"));
+
       n *= multiplier;
     }
   else if (e != LONGINT_OK)
diff --git a/tests/dd/misc.sh b/tests/dd/misc.sh
index 2c24b50..41330d0 100755
--- a/tests/dd/misc.sh
+++ b/tests/dd/misc.sh
@@ -107,4 +107,14 @@ compare err_ok err || fail=1
 
 test $fail -eq 0 && fail=$warn
 
+# Check a warning is issued for ambiguous 0x... numbers
+dd if=/dev/null count=0x1 seek=0x1 skip=0x1 status=none 2>err
+cat <<\EOF >exp
+dd: warning: '0x' is a zero multiplier; use '00x' if that is intended
+dd: warning: '0x' is a zero multiplier; use '00x' if that is intended
+dd: warning: '0x' is a zero multiplier; use '00x' if that is intended
+EOF
+compare exp err || fail=1
+
+
 Exit $fail
-- 
2.5.5