Re: dd bs= supercede ibs= and obs=

2020-02-15 Thread Ingo Schwarze
Hi,

Jan Stary wrote on Sat, Feb 15, 2020 at 11:07:04AM +0100:
> On Feb 14 17:04:51, schwa...@usta.de wrote:
>> Jason McIntyre wrote on Fri, Feb 14, 2020 at 07:28:59AM +:
>>> On Thu, Feb 13, 2020 at 11:25:07PM +0100, Jan Stary wrote:

 * Fix a factual error in the description of bs: it does not
   supersede ibs/obs, dd will error out when both are specified.

>> In fact, that's a bug in the code.  POSIX requires:
>>   bs=expr
>> Set both input and output block sizes to expr bytes,
>> superseding ibs= and obs=. 
>> (see https://pubs.opengroup.org/onlinepubs/9699919799/utilities/dd.html )

> This is the line that makes it illegal to specify bs=
> if ibs= or obs= (or bs= for that matter) has already been specified.

That appears to be part of the picture, but that is not complete.
As i read POSIX (and our manual page), bs= has to supersede even
ips= and ops= operands that follow it (rather than precede it).

Inspecting the source code, that is also how dd(1) behaved in
4.3BSD-Reno, which got its dd(1) implementation from AT&T Version 7
UNIX, which already behaved that way.

The buggy code was part of a rewrite checked into SCCS by Keith
Bostic@ on 91/07/26, i.e. after about one third of the working phase
leading from Reno to 4.4BSD.  As our Copyright notice still says,
the code he checked in was written by Keith Muller (UC San Diego).
The authorship is confirmed by an explicit entry in the file
"admin/admin/contrib/contrib".

That means the bug is at least 28.5 years old.

Regarding the fix:  It seems sufficient to just remove the four
flags causing dd(1) to exit.  The argument handling callbacks already
do the right thing: f_bs() already overrides ibs and obs, while
f_ibs() and f_obs() do nothing when bs is already set.

Even though the risk feels low for this particular change, i think
i should test this with a full make build / make release, as a
matter of principle when touching such low-level tools.

Here is the new behaviour, matching v7 and 4.3BSD-Reno:

   $ dd if=/dev/zero of=/dev/null ibs=2048 obs=1024 bs=4096 count=2 
  dd: bs supersedes ibs and obs
  2+0 records in
  2+0 records out
  8192 bytes transferred in 0.000 secs (140961886 bytes/sec)
   $ dd if=/dev/zero of=/dev/null bs=4096 ibs=2048 obs=1024 count=2 
  dd: bs supersedes ibs and obs
  2+0 records in
  2+0 records out
  8192 bytes transferred in 0.000 secs (127456319 bytes/sec)
   $ dd if=/dev/zero of=/dev/null bs=4096 count=2   
  2+0 records in
  2+0 records out
  8192 bytes transferred in 0.000 secs (230364725 bytes/sec)
   $ dd if=/dev/zero of=/dev/null ibs=4096 count=2
  2+0 records in
  16+0 records out
  8192 bytes transferred in 0.000 secs (183780146 bytes/sec)
   $ dd if=/dev/zero of=/dev/null obs=256 count=2  
  2+0 records in
  4+0 records out
  1024 bytes transferred in 0.000 secs (35021718 bytes/sec)

This also matches GNU gdd(1) from the coreutils package, fwiw.

Yours,
  Ingo


Index: args.c
===
RCS file: /cvs/src/bin/dd/args.c,v
retrieving revision 1.31
diff -u -p -r1.31 args.c
--- args.c  16 Feb 2019 10:54:00 -  1.31
+++ args.c  15 Feb 2020 13:47:04 -
@@ -68,14 +68,14 @@ static const struct arg {
void (*f)(char *);
u_int set, noset;
 } args[] = {
-   { "bs", f_bs,   C_BS,C_BS|C_IBS|C_OBS|C_OSYNC },
+   { "bs", f_bs,   C_BS,C_BS|C_OSYNC },
{ "cbs",f_cbs,  C_CBS,   C_CBS },
{ "conv",   f_conv, 0,   0 },
{ "count",  f_count,C_COUNT, C_COUNT },
{ "files",  f_files,C_FILES, C_FILES },
-   { "ibs",f_ibs,  C_IBS,   C_BS|C_IBS },
+   { "ibs",f_ibs,  C_IBS,   C_IBS },
{ "if", f_if,   C_IF,C_IF },
-   { "obs",f_obs,  C_OBS,   C_BS|C_OBS },
+   { "obs",f_obs,  C_OBS,   C_OBS },
{ "of", f_of,   C_OF,C_OF },
{ "seek",   f_seek, C_SEEK,  C_SEEK },
{ "skip",   f_skip, C_SKIP,  C_SKIP },



dd bs= supercede ibs= and obs=

2020-02-15 Thread Jan Stary
On Feb 14 17:04:51, schwa...@usta.de wrote:
> Jason McIntyre wrote on Fri, Feb 14, 2020 at 07:28:59AM +:
> > On Thu, Feb 13, 2020 at 11:25:07PM +0100, Jan Stary wrote:
> >> * Fix a factual error in the description of bs: it does not
> >>   supersede ibs/obs, dd will error out when both are specified.
> 
> In fact, that's a bug in the code.  POSIX requires:
> 
>   bs=expr
> Set both input and output block sizes to expr bytes,
> superseding ibs= and obs=. 
> 
>   (see https://pubs.opengroup.org/onlinepubs/9699919799/utilities/dd.html )

This is the line that makes it illegal to specify bs=
if ibs= or obs= (or bs= for that matter) has already been specified.

Jan


Index: args.c
===
RCS file: /cvs/src/bin/dd/args.c,v
retrieving revision 1.31
diff -u -p -r1.31 args.c
--- args.c  16 Feb 2019 10:54:00 -  1.31
+++ args.c  15 Feb 2020 10:05:25 -
@@ -68,7 +68,7 @@ static const struct arg {
void (*f)(char *);
u_int set, noset;
 } args[] = {
-   { "bs", f_bs,   C_BS,C_BS|C_IBS|C_OBS|C_OSYNC },
+   { "bs", f_bs,   C_BS,C_BS|C_OSYNC },
{ "cbs",f_cbs,  C_CBS,   C_CBS },
{ "conv",   f_conv, 0,   0 },
{ "count",  f_count,C_COUNT, C_COUNT },