Re: dd skip bug?

[Pádraig Brady]

Jim Meyering wrote:
> Pádraig Brady <address@hidden> wrote:
>> @@ -1537,10 +1581,13 @@ dd_copy (void)
>>
>>    if (skip_records != 0)
>>      {
>> -      skip (STDIN_FILENO, input_file, skip_records, input_blocksize, ibuf);
>> +      uintmax_t unskipped = skip (STDIN_FILENO, input_file,
>> +                              skip_records, input_blocksize, ibuf);
>>        /* 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.  */
>> +     call to 'skip' returns nonzero.  */
>> +      if (unskipped)
>> +    error (0, EINVAL, _("%s: cannot skip"), quote (input_file));
> 
> How about a diagnostic giving more detail?
> I.e., saying that the skip offset was larger than the length
> of the input file.

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:

$ 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?

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 ?

cheers,
Pádraig.