On 02/28/2011 01:41 AM, Pádraig Brady wrote: > 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.
Ouch. This is a pain to think about. But here are some thoughts anyway: * I went back and reread POSIX, and "dd" is allowed to issue diagnostics to stderr whenever it likes. So we don't need to worry about POSIXLY_CORRECT if all we want to do is issue diagnostics. We can issue them regardless of POSIXLY_CORRECT. * I don't understand the business with C_SYNC. People who use conv=sync know what they're doing, or ought to; there's little point giving them a warning. * For (O_DIRECT | O_CIO), surely this matters only for output_flags. If these bits are set in input_flags then O_FULLBLOCK is irrelevant, no? * If we care about max_records we should also care about skip_records, since short reads matter when skipping in a pipe, too. * Since POSIX doesn't specify the direct or cio flags, we're free to have them silently enable iflag=fullblock. But it doesn't sound right to do that. Instead, we should set conversions_mask |= C_TWOBUFS, because the input and output blocksizes might differ. * If we suggest ibs=whatever rather than iflag=fullblock, our suggestions will be portable to other POSIX implementations, which is a plus. * Rather than warn about potential problems, how about diagnosing the problems only when they actually occur? That would help us avoid crying wolf. Here's a proposed patch that tries to embody all the above, except that I haven't done the documentation (I figure we should get the behavior right first....): >From 85e3716f918e8163695a85d40fe8c561634c9e2e Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Tue, 1 Mar 2011 01:34:57 -0800 Subject: [PATCH] dd: avoid or diagnose some problems with short reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/dd.c (iread): Diagnose short reads when they mess up counts. (scanargs): If oflags=direct or oflags=cio, use C_TWOBUFS so that the output blocks are typically full. Derived from a suggestion by Pádraig Brady in: http://lists.gnu.org/archive/html/bug-coreutils/2011-02/msg00150.html --- src/dd.c | 27 ++++++++++++++++++++++++++- 1 files changed, 26 insertions(+), 1 deletions(-) diff --git a/src/dd.c b/src/dd.c index daddc1e..41ad7a3 100644 --- a/src/dd.c +++ b/src/dd.c @@ -802,7 +802,29 @@ iread (int fd, char *buf, size_t size) process_signals (); nread = read (fd, buf, size); if (! (nread < 0 && errno == EINTR)) - return nread; + { + static ssize_t prev_nread; + static bool warned; + + if (nread != 0 && 0 < prev_nread && prev_nread < size + && iread_fnc == iread + && ! (conversions_mask & C_TWOBUFS) + && (skip_records + || (0 < max_records && max_records < (uintmax_t) -1)) + && ! warned) + { + unsigned long int prev = prev_nread; + unsigned long int ibs = input_blocksize; + error (0, 0, _("warning: short read (%lu bytes)"), prev); + error (0, 0, + _("Perhaps you wanted ibs=%lu rather than bs=%lu?"), + ibs, ibs); + warned = true; + } + + prev_nread = nread; + return nread; + } } } @@ -1075,6 +1097,9 @@ scanargs (int argc, char *const *argv) conversions_mask |= C_TWOBUFS; } + if (output_flags & (O_DIRECT | O_CIO)) + conversions_mask |= C_TWOBUFS; + if (input_blocksize == 0) input_blocksize = DEFAULT_BLOCKSIZE; if (output_blocksize == 0) -- 1.7.4
