Re: [PATCH] dd - support for reading of full blocks

[Jim Meyering]

Kamil Dudka <address@hidden> wrote:
> On Monday 21 July 2008 12:53:08 you wrote:
...
> Ok, description improved. Also applied some Jim's recommendations
> from another RFE - new patch in attachment...

Thanks for adjusting it.
Here's your patch with a slightly modified log message
(an "*" goes only on the first line for a given file: the one
that lists the file's name)
followed by another to fix up some minor problems.

I'll push the two of them tomorrow.

>From ea17007da19c082c943c384c364e9551d22bd06f Mon Sep 17 00:00:00 2001
From: Kamil Dudka <address@hidden>
Date: Wed, 23 Jul 2008 11:29:21 +0200
Subject: [PATCH] dd: new option: iflag=fullblock to accumulate full input blocks

* src/dd.c (iread_fullblock): New function for reading full blocks.
(scanargs): Check for new parameter iflag=fullblock.
(skip): Use iread_fnc pointer instead of iread function.
(dd_copy): Use iread_fnc pointer instead of iread function.
* tests/dd/misc: Add test for dd - read full blocks.
* doc/coretuils.texi: Mention new parameter iflag=fullblock.
* NEWS: Mentioned the change.
---
 NEWS               |    4 ++++
 doc/coreutils.texi |    6 ++++++
 src/dd.c           |   39 +++++++++++++++++++++++++++++++++++++--
 tests/dd/misc      |    9 +++++++++
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index d6ed89e..68a27fa 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   represents the maximum number of inputs that will be merged at once.
   When processing more than NMERGE inputs, sort uses temporary files.

+  dd accepts a new parameter iflag=fullblock which turn on reading of full
+  blocks  where possible. If this parameter is used and 'read' call is
+  terminated during read, it will be called again for remainder input.
+
 ** Bug fixes

   chcon --verbose now prints a newline after each message
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 81e3b91..b95f8dc 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7719,6 +7719,12 @@ platforms that distinguish binary from text I/O.
 Use text I/O.  Like @samp{binary}, this option has no effect on
 standard platforms.

address@hidden fullblock
address@hidden fullblock
+Read full blocks from input if possible. read() may return early
+if a full block is not available, so retry until data is available
+or end of file is reached. This flag can be used only for the iflag option.
+
 @end table

 These flags are not supported on all systems, and @samp{dd} rejects
diff --git a/src/dd.c b/src/dd.c
index ead9574..1b620df 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -225,6 +225,9 @@ static sig_atomic_t volatile interrupt_signal;
 /* A count of the number of pending info signals that have been received.  */
 static sig_atomic_t volatile info_signal_count;

+/* Function used for read (to handle iflag=fullblock parameter) */
+static ssize_t (*iread_fnc) (int fd, char *buf, size_t size);
+
 /* A longest symbol in the struct symbol_values tables below.  */
 #define LONGEST_SYMBOL "fdatasync"

@@ -257,6 +260,7 @@ static struct symbol_value const conversions[] =
 };

 /* Flags, for iflag="..." and oflag="...".  */
+#define O_FULLBLOCK 010000000 /* Read only full blocks from input */
 static struct symbol_value const flags[] =
 {
   {"append",   O_APPEND},
@@ -271,6 +275,7 @@ static struct symbol_value const flags[] =
   {"nonblock", O_NONBLOCK},
   {"sync",     O_SYNC},
   {"text",     O_TEXT},
+  {"fullblock", O_FULLBLOCK}, /* Read only full blocks from input */
   {"",         0}
 };

@@ -762,6 +767,27 @@ iread (int fd, char *buf, size_t size)
     }
 }

+/* Wrapper around iread function which reads full blocks if possible */
+static ssize_t
+iread_fullblock (int fd, char *buf, size_t size)
+{
+  ssize_t nread = 0;
+
+  while (0 < size)
+    {
+      ssize_t ncurr = iread(fd, buf, size);
+      if (ncurr < 0)
+       return ncurr;
+      if (ncurr == 0)
+       break;
+      nread += ncurr;
+      buf   += ncurr;
+      size  -= ncurr;
+    }
+
+  return nread;
+}
+
 /* Write to FD the buffer BUF of size SIZE, processing any signals
    that arrive.  Return the number of bytes written, setting errno if
    this is less than SIZE.  Keep trying if there are partial
@@ -1000,6 +1026,15 @@ scanargs (int argc, char *const *argv)
   if (input_flags & (O_DSYNC | O_SYNC))
     input_flags |= O_RSYNC;

+  if (output_flags & O_FULLBLOCK)
+    {
+      error (0, 0, "%s: %s", _("invalid output flag"), "'fullblock'");
+      usage (EXIT_FAILURE);
+    }
+  iread_fnc = (input_flags & O_FULLBLOCK)?
+    iread_fullblock:
+    iread;
+
   if (multiple_bits_set (conversions_mask & (C_ASCII | C_EBCDIC | C_IBM)))
     error (EXIT_FAILURE, 0, _("cannot combine any two of {ascii,ebcdic,ibm}"));
   if (multiple_bits_set (conversions_mask & (C_BLOCK | C_UNBLOCK)))
@@ -1197,7 +1232,7 @@ skip (int fdesc, char const *file, uintmax_t records, 
size_t blocksize,

       do
        {
-         ssize_t nread = iread (fdesc, buf, blocksize);
+         ssize_t nread = iread_fnc (fdesc, buf, blocksize);
          if (nread < 0)
            {
              if (fdesc == STDIN_FILENO)
@@ -1508,7 +1543,7 @@ dd_copy (void)
                (conversions_mask & (C_BLOCK | C_UNBLOCK)) ? ' ' : '\0',
                input_blocksize);

-      nread = iread (STDIN_FILENO, ibuf, input_blocksize);
+      nread = iread_fnc (STDIN_FILENO, ibuf, input_blocksize);

       if (nread == 0)
        break;                  /* EOF.  */
diff --git a/tests/dd/misc b/tests/dd/misc
index d54fbfa..24e5eba 100755
--- a/tests/dd/misc
+++ b/tests/dd/misc
@@ -88,6 +88,15 @@ fi
 outbytes=`echo x | dd bs=3 ibs=10 obs=10 conv=sync 2>/dev/null | wc -c`
 test "$outbytes" -eq 3 || fail=1

+(echo a; sleep .1; echo b) \
+  | LC_ALL=C dd bs=4 status=noxfer iflag=fullblock >out 2>err || fail=1
+echo "a
+b" > out_ok
+echo "1+0 records in
+1+0 records out" > err_ok
+compare out out_ok || fail=1
+compare err err_ok || fail=1
+
 test $fail -eq 0 && fail=$warn

 (exit $fail); exit $fail
--
1.6.0.rc0.16.g6458d


>From ddb45f061ffe7635313fe51b042a4e9e4b623495 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Wed, 23 Jul 2008 14:50:52 +0200
Subject: [PATCH] dd: minor fullblock changes

* src/dd.c (O_FULLBLOCK): Define using an enum, not #define.
Derive the value, rather than hard-coding to one that might conflict.
(usage): Mention iflag=fullblock in --help output.
(scanargs): Reset the O_FULLBLOCK bit, so that we don't try to set
an undefined attribute via fcntl (fd, F_SETFL, ...
* tests/dd/misc: Signal framework_failure when necessary.
Use "compare actual expected", so any diffs look "right".
* NEWS (dd): Alphabetize and reword.
* coreutils.texi (dd invocation): Adjust wording.
---
 NEWS               |    8 ++++----
 doc/coreutils.texi |    8 +++++---
 src/dd.c           |   43 +++++++++++++++++++++++++++++++++++--------
 tests/dd/misc      |    9 ++++-----
 4 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/NEWS b/NEWS
index 68a27fa..8753fcf 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   comm accepts new option, --output-delimiter=STR, that allows specification
   of an output delimiter other than the default single TAB.

+  dd accepts iflag=fullblock to make it accumulate full input blocks.
+  With this new option, after a short read, dd repeatedly calls read,
+  until it fills the incomplete block, reaches EOF, or encounters an error.
+
   md5sum now accepts the new option, --quiet, to suppress the printing of
   'OK' messages.  sha1sum, sha224sum, sha384sum, and sha512sum accept it, too.

@@ -27,10 +31,6 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   represents the maximum number of inputs that will be merged at once.
   When processing more than NMERGE inputs, sort uses temporary files.

-  dd accepts a new parameter iflag=fullblock which turn on reading of full
-  blocks  where possible. If this parameter is used and 'read' call is
-  terminated during read, it will be called again for remainder input.
-
 ** Bug fixes

   chcon --verbose now prints a newline after each message
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index b95f8dc..8eb8ac9 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7721,9 +7721,11 @@ standard platforms.

 @item fullblock
 @opindex fullblock
-Read full blocks from input if possible. read() may return early
-if a full block is not available, so retry until data is available
-or end of file is reached. This flag can be used only for the iflag option.
+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 can be used only with @code{iflag}.

 @end table

diff --git a/src/dd.c b/src/dd.c
index 1b620df..a1fafb8 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -225,7 +225,7 @@ static sig_atomic_t volatile interrupt_signal;
 /* A count of the number of pending info signals that have been received.  */
 static sig_atomic_t volatile info_signal_count;

-/* Function used for read (to handle iflag=fullblock parameter) */
+/* Function used for read (to handle iflag=fullblock parameter).  */
 static ssize_t (*iread_fnc) (int fd, char *buf, size_t size);

 /* A longest symbol in the struct symbol_values tables below.  */
@@ -259,8 +259,32 @@ static struct symbol_value const conversions[] =
   {"", 0}
 };

+enum
+  {
+    /* Use a value that is larger than that of any other O_ symbol.  */
+    O_FULLBLOCK = ((MAX (O_APPEND,
+                   MAX (O_BINARY,
+                   MAX (O_DIRECT,
+                   MAX (O_DIRECTORY,
+                   MAX (O_DSYNC,
+                   MAX (O_NOATIME,
+                   MAX (O_NOCTTY,
+                   MAX (O_NOFOLLOW,
+                   MAX (O_NOLINKS,
+                   MAX (O_NONBLOCK,
+                   MAX (O_SYNC, O_TEXT)))))))))))) << 1)
+  };
+
+/* Ensure that we didn't shift it off the end.  */
+verify (O_FULLBLOCK != 0);
+
+/* Ensure that this is a single-bit value.  */
+verify ((O_FULLBLOCK &
+        ( O_APPEND | O_BINARY | O_DIRECT | O_DIRECTORY | O_DSYNC
+          | O_NOATIME | O_NOCTTY | O_NOFOLLOW | O_NOLINKS | O_NONBLOCK
+          | O_SYNC | O_TEXT)) == 0);
+
 /* Flags, for iflag="..." and oflag="...".  */
-#define O_FULLBLOCK 010000000 /* Read only full blocks from input */
 static struct symbol_value const flags[] =
 {
   {"append",   O_APPEND},
@@ -275,7 +299,7 @@ static struct symbol_value const flags[] =
   {"nonblock", O_NONBLOCK},
   {"sync",     O_SYNC},
   {"text",     O_TEXT},
-  {"fullblock", O_FULLBLOCK}, /* Read only full blocks from input */
+  {"fullblock", O_FULLBLOCK}, /* Accumulate full blocks from input.  */
   {"",         0}
 };

@@ -493,6 +517,8 @@ Each FLAG symbol may be:\n\
        fputs (_("  dsync     use synchronized I/O for data\n"), stdout);
       if (O_SYNC)
        fputs (_("  sync      likewise, but also for metadata\n"), stdout);
+      fputs (_("  fullblock  accumulate full blocks of input (iflag only)\n"),
+            stdout);
       if (O_NONBLOCK)
        fputs (_("  nonblock  use non-blocking I/O\n"), stdout);
       if (O_NOATIME)
@@ -767,7 +793,7 @@ iread (int fd, char *buf, size_t size)
     }
 }

-/* Wrapper around iread function which reads full blocks if possible */
+/* Wrapper around iread function to accumulate full blocks.  */
 static ssize_t
 iread_fullblock (int fd, char *buf, size_t size)
 {
@@ -775,7 +801,7 @@ iread_fullblock (int fd, char *buf, size_t size)

   while (0 < size)
     {
-      ssize_t ncurr = iread(fd, buf, size);
+      ssize_t ncurr = iread (fd, buf, size);
       if (ncurr < 0)
        return ncurr;
       if (ncurr == 0)
@@ -1031,9 +1057,10 @@ scanargs (int argc, char *const *argv)
       error (0, 0, "%s: %s", _("invalid output flag"), "'fullblock'");
       usage (EXIT_FAILURE);
     }
-  iread_fnc = (input_flags & O_FULLBLOCK)?
-    iread_fullblock:
-    iread;
+  iread_fnc = ((input_flags & O_FULLBLOCK)
+              ? iread_fullblock
+              : iread);
+  input_flags &= ~O_FULLBLOCK;

   if (multiple_bits_set (conversions_mask & (C_ASCII | C_EBCDIC | C_IBM)))
     error (EXIT_FAILURE, 0, _("cannot combine any two of {ascii,ebcdic,ibm}"));
diff --git a/tests/dd/misc b/tests/dd/misc
index 24e5eba..e550d6a 100755
--- a/tests/dd/misc
+++ b/tests/dd/misc
@@ -90,12 +90,11 @@ test "$outbytes" -eq 3 || fail=1

 (echo a; sleep .1; echo b) \
   | LC_ALL=C dd bs=4 status=noxfer iflag=fullblock >out 2>err || fail=1
-echo "a
-b" > out_ok
+printf 'a\nb\n' > out_ok || framework_failure
 echo "1+0 records in
-1+0 records out" > err_ok
-compare out out_ok || fail=1
-compare err err_ok || fail=1
+1+0 records out" > err_ok || framework_failure
+compare out_ok out || fail=1
+compare err_ok err || fail=1

 test $fail -eq 0 && fail=$warn

--
1.6.0.rc0.16.g6458d