On 01/03/11 21:40, Pádraig Brady wrote:
> On 01/03/11 17:45, Paul Eggert wrote:
>> On 03/01/2011 03:27 AM, Pádraig Brady wrote:
>>
>>> So the standard way to accumulate short reads to a full write,
>>> is to specify separate ibs and obs (we'd probably want to prompt about
>>> setting obs too for efficiency)
>>
>> Yes, good point, the diagnostic should suggest ibs=N obs=N
>> (instead of just ibs=N).
>>
>> By the way, the relationship between fullblock and ibs=N obs=N is
>> a curious one, one that I don't fully understand.  If you have
>> ibs=N obs=N, why would you need fullblock?  This should probably
>> be documented (preferably by someone who understands it :-).
> 
> Well as I understand it, it's to do with 'count'.
> count refers to the number of input reads,
> both partial and full.
> 
> So the advice to use iflag=fullblock is probably safer,
> especially when a count (or skip) is specified.

Thinking about it more, we should at least split up the patch.
So for the oflag=direct case the attached just enables fullblock
(as using C_TWOBUFS would require more mem, CPU, and also messes
up if the user specified a count).

I'm not sure we should try to be more clever than this,
and accept that dd is a low level tool that can be
used in a myriad of ways.

cheers,
Pádraig.
>From be0af61dd2288f1e6df4e28d6d955395e3780e7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Fri, 25 Feb 2011 12:27:25 +0000
Subject: [PATCH] dd: enable iflag=fullblock for oflag=direct or oflag=cio

* NEWS: Mention the change in behavior.
* doc/coreutils.texi: Document when iflag=fullblock is implied.
* src/dd.c (scan_args): Enable O_FULLBLOCK when needed.
---
 NEWS               |    5 +++++
 doc/coreutils.texi |    1 +
 src/dd.c           |    8 ++++++++
 3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index a367d8d..8627a0b 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   delimiter and an unbounded range like "-f1234567890-".
   [bug introduced in coreutils-5.3.0]
 
+** Changes in behavior
+
+  dd now enables iflag=fullblock with oflag=direct or oflag=cio
+  where short reads can have adverse effects.
+
 
 * Noteworthy changes in release 8.10 (2011-02-04) [stable]
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index ea35afe..5df2386 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8216,6 +8216,7 @@ 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.
+This flag is implied with @samp{oflag=cio} or @samp{oflag=direct}.
 This flag can be used only with @code{iflag}.
 
 @end table
diff --git a/src/dd.c b/src/dd.c
index daddc1e..b2f4bf3 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1085,6 +1085,14 @@ scanargs (int argc, char *const *argv)
   if (input_flags & (O_DSYNC | O_SYNC))
     input_flags |= O_RSYNC;
 
+  /* Enable 'fullblock' with 'direct' or 'cio' as we don't want to
+     write partial blocks to output, which would disable O_DIRECT. An
+     alternative would be to enable C_TWOBUFS to accumulate full output
+     blocks.  However that wouldn't work when a count is specified, and
+     is also less efficient.  */
+  if (output_flags & (O_DIRECT | O_CIO))
+    input_flags |= O_FULLBLOCK;
+
   if (output_flags & O_FULLBLOCK)
     {
       error (0, 0, "%s: %s", _("invalid output flag"), "'fullblock'");
-- 
1.7.4

Reply via email to