Re: [PATCH] Don't do unneccesary memory copies in dd.

[Pádraig Brady]

Jim Meyering wrote:
> Paul Eggert <address@hidden> wrote:
>>
>> The simplest fix I see is to revert the patch.  Of course one could up
>> with something fancier....
> 
> Good catch!  Thank you.
> I've done that, and added a test for the required behavior.
> 
>>From 80d42899ec71f237b65f5c974e0a2c1b9c121e09 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Fri, 21 Nov 2008 23:17:44 +0100
> Subject: [PATCH 1/2] Revert "dd: avoid unnecessary memory copies"
> 
> This reverts commit fbd87029cfc494a72bb73ade27ef46382c5bc832.
> Paul Eggert noticed that the offending was wrong:
> http://lists.gnu.org/archive/html/bug-coreutils/2008-11/msg00153.html

Paul is correct as usual.

But I'd like to use the attached patch instead of your PATCH 1,
because I think it simplifies the code and makes it explicit
in the code and to users reading the docs what's happening in
this case. It also more closely conforms to POSIX I think
in the unusual case where both bs= and [io]bs= are specified.

thanks,
Pádraig.

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 935129f..8461a16 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7573,6 +7573,9 @@ This makes @command{dd} write @var{bytes} per block.
 Set both input and output block sizes to @var{bytes}.
 This makes @command{dd} read and write @var{bytes} per block,
 overriding any @samp{ibs} and @samp{obs} settings.
+In addition, if no data transforming @option{conv} options are
+specified, each input block is copied to the output as a single
+block without aggregating short reads.
 
 @item address@hidden
 @opindex cbs
diff --git a/src/dd.c b/src/dd.c
index e1e38e9..e98ede4 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -455,7 +455,7 @@ Usage: %s [OPERAND]...\n\
       fputs (_("\
 Copy a file, converting and formatting according to the operands.\n\
 \n\
-  bs=BYTES        force ibs=BYTES and obs=BYTES\n\
+  bs=BYTES        read and write BYTES bytes at a time\n\
   cbs=BYTES       convert BYTES bytes at a time\n\
   conv=CONVS      convert the file as per the comma separated symbol list\n\
   count=BLOCKS    copy only BLOCKS input blocks\n\
@@ -1033,13 +1033,15 @@ scanargs (int argc, char *const *argv)
 
   if (blocksize)
     input_blocksize = output_blocksize = blocksize;
+  else
+    /* POSIX states we aggregate short reads
+       into output_blocksize if bs= is not specified.  */
+    conversions_mask |= C_TWOBUFS;
 
   if (input_blocksize == 0)
     input_blocksize = DEFAULT_BLOCKSIZE;
   if (output_blocksize == 0)
     output_blocksize = DEFAULT_BLOCKSIZE;
-  if (input_blocksize != output_blocksize)
-    conversions_mask |= C_TWOBUFS;
   if (conversion_blocksize == 0)
     conversions_mask &= ~(C_BLOCK | C_UNBLOCK);