Subject: Re: bug#13391: dd silently ignores lseek error
From: Pádraig Brady <[email-masked]>
To: Paul Eggert <[email-masked]>
Cc: Neil Klopfenstein <[email-masked]>, [email-masked]
Date: Wed, 09 Jan 2013 01:14:22 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

On 01/08/2013 08:55 PM, Paul Eggert wrote:
> On 01/08/13 10:11, Neil Klopfenstein wrote:
>> Note that it begins reading at the _beginning of the ar file_ -- the 'skip'
>> argument has failed silently.
>
> But the 'skip' hasn't failed.  It's merely being implemented via 'read'
> rather than via 'lseek'.  The records are being skipped correctly.
>
> It might be useful to give dd a new option, which causes it
> to insist on lseeking rather than reading in cases like these,
> and to report an error if the lseek fails.

I had a look around for a tool to verify
that a file/device supports the seek operation
and couldn't find one.
So this seems like useful functionality.
Worth applying the attached?

thanks,
Pádraig.
From ea524ab7388bb35e591dcdb0fc7f7989d61143ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Wed, 9 Jan 2013 00:42:38 +0000
Subject: [PATCH] dd: add [io]flag=seekable to verify file support for lseek

* src/dd.c: Add the new O_SEEKABLE flag.
(main): Verify leek() works if O_SEEKABLE is set.
(usage): Describe the new flag.
* tests/dd/misc.sh: Augment the test for the new options.
* doc/coreutils.texi (dd invocation): Describe the new option.
* cfg.mk (sc_dd_O_FLAGS): Add O_SEEKABLE to the list of private
flags with a a single underscore.
* NEWS: Mention the new feature.
Suggested by Paul Eggert in http://bugs.gnu.org/13391
---
 NEWS               |    3 +++
 cfg.mk             |    2 +-
 doc/coreutils.texi |    6 ++++++
 src/dd.c           |   28 +++++++++++++++++++++++++++-
 tests/dd/misc.sh   |    8 ++++++++
 5 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 754b2cf..9c415ec 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** New features
 
+  dd now accepts 'iflag=seekable' and 'oflag=seekable' to verify that the
+  corresponding files support the seek operation.
+
   df now accepts the --output[=FIELD_LIST] option to define the list of columns
   to include in the output, or all available columns if the FIELD_LIST is
   omitted.  Note this enables df to output both block and inode fields together.
diff --git a/cfg.mk b/cfg.mk
index fbc64b4..bf43861 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -57,7 +57,7 @@ _hv_file ?= $(srcdir)/tests/misc/help-version
 dd = $(srcdir)/src/dd.c
 sc_dd_O_FLAGS:
 	@rm -f $@.1 $@.2
-	@{ echo O_FULLBLOCK; echo O_NOCACHE;				\
+	@{ echo O_FULLBLOCK; echo O_NOCACHE; echo O_SEEKABLE;		\
 	  perl -nle '/^ +\| (O_\w*)$$/ and print $$1' $(dd); } | sort > $@.1
 	@{ echo O_NOFOLLOW; perl -nle '/{"[a-z]+",\s*(O_\w+)},/ and print $$1' \
 	  $(dd); } | sort > $@.2
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 45a4b3d..9ee9d86 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8399,6 +8399,12 @@ rather than a block count, which allows specifying
 an offset that is not a multiple of the I/O block size.
 This flag can be used only with @code{oflag}.
 
+@item seekable
+@opindex seekable
+Fail unless the file is seekable.
+Note @samp{skip=} or @samp{seek=} don't suffice for this check,
+as they will resort to reading, to skip over data.
+
 @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 ef5664b..21b4b21 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -216,6 +216,9 @@ static xtime_t start_time;
 /* True if input is seekable.  */
 static bool input_seekable;
 
+/* True if output is seekable.  */
+static bool output_seekable;
+
 /* Error number corresponding to initial attempt to lseek input.
    If ESPIPE, do not issue any more diagnostics about it.  */
 static int input_seek_errno;
@@ -326,7 +329,10 @@ enum
     O_SKIP_BYTES = FFS_MASK (v4),
     v5 = v4 ^ O_SKIP_BYTES,
 
-    O_SEEK_BYTES = FFS_MASK (v5)
+    O_SEEK_BYTES = FFS_MASK (v5),
+    v6 = v5 ^ O_SEEK_BYTES,
+
+    O_SEEKABLE = FFS_MASK (v6)
   };
 
 /* Ensure that we got something.  */
@@ -335,6 +341,7 @@ verify (O_NOCACHE != 0);
 verify (O_COUNT_BYTES != 0);
 verify (O_SKIP_BYTES != 0);
 verify (O_SEEK_BYTES != 0);
+verify (O_SEEKABLE != 0);
 
 #define MULTIPLE_BITS_SET(i) (((i) & ((i) - 1)) != 0)
 
@@ -344,6 +351,7 @@ verify ( ! MULTIPLE_BITS_SET (O_NOCACHE));
 verify ( ! MULTIPLE_BITS_SET (O_COUNT_BYTES));
 verify ( ! MULTIPLE_BITS_SET (O_SKIP_BYTES));
 verify ( ! MULTIPLE_BITS_SET (O_SEEK_BYTES));
+verify ( ! MULTIPLE_BITS_SET (O_SEEKABLE));
 
 /* Flags, for iflag="..." and oflag="...".  */
 static struct symbol_value const flags[] =
@@ -366,6 +374,7 @@ static struct symbol_value const flags[] =
   {"count_bytes", O_COUNT_BYTES},
   {"skip_bytes",  O_SKIP_BYTES},
   {"seek_bytes",  O_SEEK_BYTES},
+  {"seekable",    O_SEEKABLE},
   {"",		0}
 };
 
@@ -619,6 +628,9 @@ Each FLAG symbol may be:\n\
       if (O_SEEK_BYTES)
         fputs (_("  seek_bytes  treat 'seek=N' as a byte count (oflag only)\n\
 "), stdout);
+      if (O_SEEKABLE)
+        fputs (_("  seekable  fail unless seekable\n\
+"), stdout);
 
       {
         char const *siginfo_name = (SIGINFO == SIGUSR1 ? "USR1" : "INFO");
@@ -1607,6 +1619,7 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
         }
       /* else file_size && offset > OFF_T_MAX or file ! seekable */
 
+
       do
         {
           ssize_t nread = iread_fnc (fdesc, buf, records ? blocksize : *bytes);
@@ -2270,6 +2283,19 @@ main (int argc, char **argv)
         }
     }
 
+  offset = lseek (STDOUT_FILENO, 0, SEEK_CUR);
+  output_seekable = (0 <= offset);
+  if (! input_seekable && (input_flags & O_SEEKABLE))
+    {
+      error (EXIT_FAILURE, input_seek_errno, _("input is not seekable %s"),
+             quote (input_file));
+    }
+  if (! output_seekable && (output_flags & O_SEEKABLE))
+    {
+      error (EXIT_FAILURE, errno, _("output is not seekable %s"),
+             quote (output_file));
+    }
+
   start_time = gethrxtime ();
 
   exit_status = dd_copy ();
diff --git a/tests/dd/misc.sh b/tests/dd/misc.sh
index b9ad31a..cc84457 100755
--- a/tests/dd/misc.sh
+++ b/tests/dd/misc.sh
@@ -102,4 +102,12 @@ compare err_ok err || fail=1
 
 test $fail -eq 0 && fail=$warn
 
+# check iflag=seekable and oflag=seekable
+dd if=/dev/null of=/dev/null iflag=seekable oflag=seekable count=0 || fail=1
+echo 'pipe' | dd of=/dev/null iflag=seekable count=0 && fail=1
+dd if=/dev/null oflag=seekable count=0 status=none 2>err | cat
+echo "dd: output is not seekable 'standard output': Illegal seek" > err_ok ||
+  framework_failure_
+compare err_ok err || fail=1
+
 Exit $fail
-- 
1.7.6.4