[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: dd skip bug?
From: |
Pádraig Brady |
Subject: |
Re: dd skip bug? |
Date: |
Tue, 27 Jan 2009 11:21:08 +0000 |
User-agent: |
Thunderbird 2.0.0.6 (X11/20071008) |
Jim Meyering wrote:
> Pádraig Brady <address@hidden> wrote:
> ...
>> I changed message as shown below.
>> But in further testing before commiting I noticed an inconsistency
>> with my fix for this case, specifically when dealing with blocksizes > 1.
>>
>> For seekable files I count a partial block as an unread record,
>> thus printing the diagnostic. This is not the case for pipes though
>> as demonstrated here:
>
> Good catch.
>
>> $ truncate -s3 pb.size
>>
>> $ dd iflag=fullblock bs=2 skip=2 if=pb.size
>> ./dd: `pb.size': skip offset was past end of input file: Invalid argument
>>
>> $ cat pb.size | ./dd iflag=fullblock bs=2 skip=2
>>
>> $ cat pb.size | ./dd iflag=fullblock bs=1 skip=4
>> ./dd: `standard input': skip offset was past end of input file: Invalid
>> argument
>>
>> So should the first dd command not print the message,
>> or should the second dd command print the message?
>
> Whatever you prefer, as long as it's consistent.
Well it's more awkward to output the warning for all cases,
as currently we can't distinguish between short reads from
a pipe, and a partial block at the end of the stream.
Thus the error message ".. past end of input file"
is not appropriate for both cases.
Therefore I split out the 2 error cases in the patch below.
It's more consistent but I'm a little worried it's too involved.
>> I leaning towards changing skip() for seekable files
>> to return the number of _full_ records unskipped
>> to keep it the same as for pipes.
>> Then printing the warning for all cases where
>> the input offset != skip*bs ?
Note you originally added input_offset in 6f31b14a, to deal with
skipping over unreadable parts of _seekable_ files. However I don't
see any problem overloading it to record the current offset of all inputs?
>
> But remember that the initial offset for a seekable file
> need not be zero. I.e., this should now evoke a warning:
>
> printf 1234 > k && (dd bs=1 skip=2 count=0 && dd bs=1 skip=3) < k
Yep, that was missed in my last patch, but fixed in the patch below
which passes the following:
# Turn dd skip past EOF warnings into errors to simplify testing
DD() {
./dd status=noxfer iflag=fullblock $* 2>&1 |
grep "skip offset was past end of input file" >/dev/null\
&& return 1 || return 0
}
./truncate -s3 dd.three
DD bs=1 skip=3 if=dd.three || fail=1
DD bs=2 skip=2 if=dd.three && fail=1
DD bs=2 skip=3 if=dd.three && fail=1
cat dd.three | DD bs=1 skip=3 || fail=1
cat dd.three | DD bs=2 skip=2 && fail=1
cat dd.three | DD bs=2 skip=3 && fail=1
(DD bs=1 skip=1 count=0 && DD bs=1 skip=2) <dd.three || fail=1
(DD bs=1 skip=1 count=0 && DD bs=1 skip=3) <dd.three && fail=1
cheers,
Pádraig
diff --git a/src/dd.c b/src/dd.c
index d683c5d..4184297 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -200,8 +200,7 @@ static bool input_seekable;
static int input_seek_errno;
/* File offset of the input, in bytes, along with a flag recording
- whether it overflowed. The offset is valid only if the input is
- seekable and if the offset has not overflowed. */
+ whether it overflowed. */
static uintmax_t input_offset;
static bool input_offset_overflow;
@@ -1259,12 +1258,62 @@ skip (int fdesc, char const *file, uintmax_t records,
size_t blocksize,
&& 0 <= skip_via_lseek (file, fdesc, offset, SEEK_CUR))
{
if (fdesc == STDIN_FILENO)
- advance_input_offset (offset);
- return 0;
+ {
+ struct stat st;
+ if (fstat (STDIN_FILENO, &st) != 0)
+ error (EXIT_FAILURE, errno, _("cannot fstat %s"), quote (file));
+ if (S_ISREG (st.st_mode) && st.st_size < (input_offset + offset))
+ {
+ /* When skipping past EOF, return the number of _full_ blocks
+ * that are not skipped, and set offset to EOF, so the caller
+ * can determine the requested skip was not satisfied. */
+ records = ( offset - st.st_size ) / blocksize;
+ offset = st.st_size - input_offset;
+ }
+ else
+ records = 0;
+ advance_input_offset (offset);
+ }
+ else
+ records = 0;
+ return records;
}
else
{
int lseek_errno = errno;
+ off_t soffset;
+
+ /* The seek request may have failed above if it was too big
+ (> device size, > max file size, etc.)
+ Or it may not have been done at all (> OFF_T_MAX).
+ Therefore try to seek to the end of the file,
+ to avoid redundant reading. */
+ if ((soffset = skip_via_lseek (file, fdesc, 0, SEEK_END)) >= 0)
+ {
+ /* File is seekable, and we're at the end of it, and
+ size <= OFF_T_MAX. So there's no point using read to advance. */
+
+ if (!lseek_errno)
+ {
+ /* The original seek was not attempted as offset > OFF_T_MAX.
+ We should error for write as can't get to the desired
+ location, even if OFF_T_MAX < max file size.
+ For read we're not going to read any data anyway,
+ so we should error for consistency.
+ It would be nice to not error for /dev/{zero,null}
+ for any offset, but that's not a significant issue. */
+ lseek_errno = EOVERFLOW;
+ }
+
+ if (fdesc == STDIN_FILENO)
+ error (0, lseek_errno, _("%s: cannot skip"), quote (file));
+ else
+ error (0, lseek_errno, _("%s: cannot seek"), quote (file));
+ /* If the file has a specific size and we've asked
+ to skip/seek beyond the max allowable, then quit. */
+ quit (EXIT_FAILURE);
+ }
+ /* else file_size && offset > OFF_T_MAX or file ! seekable */
do
{
@@ -1537,10 +1586,28 @@ dd_copy (void)
if (skip_records != 0)
{
- skip (STDIN_FILENO, input_file, skip_records, input_blocksize, ibuf);
+ uintmax_t us_bytes = input_offset + (skip_records * input_blocksize);
+ uintmax_t us_blocks = skip (STDIN_FILENO, input_file,
+ skip_records, input_blocksize, ibuf);
+ us_bytes -= input_offset;
+
/* POSIX doesn't say what to do when dd detects it has been
- asked to skip past EOF, so I assume it's non-fatal if the
- call to 'skip' returns nonzero. FIXME: maybe give a warning. */
+ asked to skip past EOF, so I assume it's non-fatal. */
+ if (us_blocks || (!input_offset_overflow && us_bytes))
+ {
+ if (!us_blocks && !input_seekable && !(iread_fnc == iread_fullblock))
+ {
+ error (0, 0,
+ _("%s: unable to skip to requested offset"),
+ quote (input_file));
+ }
+ else
+ {
+ error (0, EINVAL,
+ _("%s: skip offset was past end of input file"),
+ quote (input_file));
+ }
+ }
}
if (seek_records != 0)
@@ -1778,7 +1845,7 @@ main (int argc, char **argv)
offset = lseek (STDIN_FILENO, 0, SEEK_CUR);
input_seekable = (0 <= offset);
- input_offset = offset;
+ input_offset = MAX(0, offset);
input_seek_errno = errno;
if (output_file == NULL)
- Re: dd skip bug?, Pádraig Brady, 2009/01/15
- Re: dd skip bug?, Pádraig Brady, 2009/01/23
- Re: dd skip bug?, Jim Meyering, 2009/01/24
- Re: dd skip bug?, Pádraig Brady, 2009/01/26
- Re: dd skip bug?, Jim Meyering, 2009/01/27
- Re: dd skip bug?,
Pádraig Brady <=
- Re: dd skip bug?, Pádraig Brady, 2009/01/27
- Re: dd skip bug?, Jim Meyering, 2009/01/27
- Re: dd skip bug?, Pádraig Brady, 2009/01/27