Subject: Re: bug#7362: dd strangeness

From: Pádraig Brady <[email-masked]>
Date: Mon, 28 Feb 2011 09:41:11 +0000

On 25/02/11 17:29, Paul Eggert wrote:
> On 02/25/2011 04:19 AM, Pádraig Brady wrote:
> 
>> Attached is a proposed solution to this.
> 
> My kneejerk reaction is that it tries to do too much inferring,
> and ends up being more complicated than giving the user more control.
> 
> If we're going to change the default to be not compatible with POSIX,
> we need to give the user a way to get the POSIX behavior, something
> that's less subtle than POSIXLY_CORRECT.  I suggest that we add
> a new option that is the inverse of "fullblock".  We can call it
> "partblock", say.
> 
> Then, we can say that the default is "fullblock" normally, but it is
> "partblock" if POSIXLY_CORRECT and if bs= is given and if no conversions
> other than sync, noerror, and notrunk are given.
> 
> Anyway, I'm just thinking out loud to some extent, and further
> comments are welcome.

Hmm, it's better to be explicit but I think defaulting to "fullblock"
is too risky. As an interim step at least, how about just warning
as per the attached.

cheers,
Pádraig.
>From 987438bfe5f7a38b7736f0aaca9d8377b4281408 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Fri, 25 Feb 2011 12:27:25 +0000
Subject: [PATCH] dd: suggest iflag=fullblock where appropriate

* NEWS: Mention the change in behavior.
* doc/coreutils.texi: Document when iflag=fullblock
is auto suggested.
 src/dd.c (scan_args): Suggest O_FULLBLOCK when probably needed.
---
 NEWS               |    6 ++++++
 doc/coreutils.texi |    3 +++
 src/dd.c           |   32 ++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index a367d8d..e82bc02 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,12 @@ GNU coreutils NEWS                                    -*- outline -*-
   delimiter and an unbounded range like "-f1234567890-".
   [bug introduced in coreutils-5.3.0]
 
+** Changes in behavior
+
+  dd now suggests using iflag=fullblock with oflag=direct or conv=sync
+  where short reads can have adverse effects.  This option is also
+  suggested if the user indicates a specific amount of data to read.
+
 
 * Noteworthy changes in release 8.10 (2011-02-04) [stable]
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index ea35afe..da8e80b 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8216,6 +8216,9 @@ Accumulate full blocks from input.  The @code{read} system call
 may return early if a full block is not available.
 When that happens, continue calling @code{read} to fill the remainder
 of the block.
+Using this flag is suggested when @samp{cio}, @samp{direct}
+or @samp{conv=sync} are used, or when a specific amount of input
+data is specfied.
 This flag can be used only with @code{iflag}.
 
 @end table
diff --git a/src/dd.c b/src/dd.c
index daddc1e..1a0e177 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1075,6 +1075,38 @@ scanargs (int argc, char *const *argv)
       conversions_mask |= C_TWOBUFS;
     }
 
+  if (!(input_flags & O_FULLBLOCK) && !getenv ("POSIXLY_CORRECT"))
+    {
+      bool fb_warned = false;
+      /* Suggest 'fullblock' as one wouldn't want random
+         padding applied, when reading from a pipe for example.  */
+      if (conversions_mask & C_SYNC)
+        {
+          error (0, 0, _("warning: 'iflag=fullblock' is suggested when "
+                         "padding input blocks"));
+          fb_warned = true;
+        }
+      /* Suggest 'fullblock' with 'direct' or 'cio' as again if reading from
+         a pipe, we're constrained in how we write to output.  */
+      if ((input_flags | output_flags) & (O_DIRECT | O_CIO))
+        {
+          error (0, 0, _("warning: 'iflag=fullblock' is suggested when "
+                         "writing with direct I/O constraints"));
+          fb_warned = true;
+        }
+      /* Suggest 'fullblock' if we're reading a specific number of blocks,
+         with a specific block size.  */
+      if (max_records && max_records != (uintmax_t) -1 && input_blocksize)
+        {
+          error (0, 0, _("warning: 'iflag=fullblock' is suggested when "
+                         "reading a specific amount"));
+          fb_warned = true;
+        }
+
+      if (fb_warned)
+        error (0, 0, _("Set POSIXLY_CORRECT to disable this warning"));
+    }
+
   if (input_blocksize == 0)
     input_blocksize = DEFAULT_BLOCKSIZE;
   if (output_blocksize == 0)
-- 
1.7.4