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

[Jérémy Compostella]

> Hmm, shouldn't there be a seek_bytes param too for consistency?
That was effectively my first mail question. As you talk about it in
your explanation addition in coreutils.texi I guess I should start
implementing it ?

> 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.
OK.

> 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:
OK. I would add something about count_bytes too, no ?

> 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;
>     }
OK.

> 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

Should I import your patch suggestions in my patch or do you will make
an additional commit on top of it ? That's particularly relevant if I
implement the seek_bytes operand.

Cheers,

Jérémy