Re: dd: add 'skip_bytes' and 'count_bytes' operands

[Pádraig Brady]

On 02/06/2012 02:01 PM, Jérémy Compostella wrote:
> Pádraig, all,
> 
> I took into account general comments on my commits. I attached the
> improved patch for feature. Improvements are:
> - Add by file description,
> - I put back accents on my name (Jérémy instead of Jeremy),

Great thanks.

Hmm, shouldn't there bee a seek_bytes param too for consistency?


I'd change the NEWS to something simpler like:
** New features
  dd now accepts the skip_bytes and count_bytes parameters,
  to more easily allow processing portions of a file.

This additional doc change is necessary I think:
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8354,8 +8354,10 @@ should not be too large---values larger than a few 
megabytes
 are generally wasteful or (as in the gigabyte..exabyte case) downright
 counterproductive or error-inducing.

-Use different @command{dd} invocations to use different block sizes for
-skipping and I/O@.  For example, the following shell commands copy data
+To use different block sizes for skipping and I/O@
+you can use the @samp{skip_bytes} and @samp{seek_bytes} options,
+or the traditional method of separate @command{dd} invocations.
+For example, the following shell commands copy data
 in 512 KiB blocks between a disk and a tape, but do not save or restore a
 4 KiB label at the start of the disk:

Your change to skip() looks good, but I would suggest the
following style adjustment.
diff --git a/src/dd.c b/src/dd.c
index 7c65653..6798725 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1533,12 +1533,7 @@ skip (int fdesc, char const *file, uintmax_t records, 
size_t blocksize,

       do
         {
-          ssize_t nread;
-          if (records != 0)
-            nread = iread_fnc (fdesc, buf, blocksize);
-          else
-            nread = iread_fnc (fdesc, buf, bytes);
-
+          ssize_t nread = iread_fnc (fdesc, buf, records ? blocksize : bytes);
           if (nread < 0)
             {
               if (fdesc == STDIN_FILENO)
@@ -1557,11 +1552,11 @@ skip (int fdesc, char const *file, uintmax_t records, 
size_t blocksize,
             advance_input_offset (nread);

           if (records != 0)
-            --records;
+            records--;
           else
             bytes = 0;
         }
-      while (records != 0 || bytes != 0);
+      while (records || bytes);

       return records;
     }



The test doesn't exercise the above, so I added
an extra part to do so. I also removed redundant
subshells, and used framework_failure_ where appropriate:
# count_bytes
echo 0123456789abcdefghijklm > in || framework_failure_
dd count_bytes=14 conv=swab < in > out 2> /dev/null || fail=1
case `cat out` in
  1032547698badc) ;;
  *) fail=1 ;;
esac

# skip_bytes
echo 0123456789abcdefghijklm > in || framework_failure_
dd skip_bytes=10 < in > out 2> /dev/null || fail=1
case `cat out` in
  abcdefghijklm) ;;
  *) fail=1 ;;
esac

# skip records and bytes from pipe
echo 0123456789abcdefghijklm |
  dd skip_bytes=10 bs=2 > out 2> /dev/null || fail=1
case `cat out` in
  abcdefghijklm) ;;
  *) fail=1 ;;
esac


cheers,
Pádraig