Re: [PATCH] dd: support iflag=direct with arbitrary sized files

[Pádraig Brady]

On 16/10/17 00:15, Pádraig Brady wrote:
> * src/dd.c (iread): Handle read error with a non-aligned
> file offset in the O_DIRECT case.
> * tests/dd/direct.sh: Test the iflag=direct case also.
> * NEWS: Mention the improvement.
> ---
>  NEWS               |  4 ++++
>  src/dd.c           | 15 +++++++++++----
>  tests/dd/direct.sh |  4 ++--
>  3 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index c61e4e8..00cf6df 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -16,6 +16,10 @@ GNU coreutils NEWS                                    -*- 
> outline -*-
>    to attempt to hide the original length of the file name.
>    [bug introduced in coreutils-8.28]
>  
> +** Improvements
> +
> +  dd now supports iflag=direct with arbitrary sized files.
> +
>  ** Build-related
>  
>    Default man pages are now distributed which are used if perl is
> diff --git a/src/dd.c b/src/dd.c
> index 636fac6..613dfe0 100644
> --- a/src/dd.c
> +++ b/src/dd.c
> @@ -1108,11 +1108,21 @@ static ssize_t
>  iread (int fd, char *buf, size_t size)
>  {
>    ssize_t nread;
> +  static ssize_t prev_nread;
>  
>    do
>      {
>        process_signals ();
>        nread = read (fd, buf, size);
> +      /* Ignore final read error with iflag=direct as that
> +         returns EINVAL due to the non aligned file offset.  */
> +      if (nread == -1 && errno == EINVAL
> +          && 0 < prev_nread && prev_nread < size
> +          && (input_flags & O_DIRECT))
> +        {
> +          errno = 0;
> +          nread = 0;
> +        }
>      }
>    while (nread < 0 && errno == EINTR);
>  
> @@ -1122,8 +1132,6 @@ iread (int fd, char *buf, size_t size)
>  
>    if (0 < nread && warn_partial_read)
>      {
> -      static ssize_t prev_nread;
> -
>        if (0 < prev_nread && prev_nread < size)
>          {
>            uintmax_t prev = prev_nread;
> @@ -1136,10 +1144,9 @@ iread (int fd, char *buf, size_t size)
>                     prev);
>            warn_partial_read = false;
>          }
> -
> -      prev_nread = nread;
>      }
>  
> +  prev_nread = nread;
>    return nread;
>  }
>  
> diff --git a/tests/dd/direct.sh b/tests/dd/direct.sh
> index e440ba3..f6bc9ca 100755
> --- a/tests/dd/direct.sh
> +++ b/tests/dd/direct.sh
> @@ -1,5 +1,5 @@
>  #!/bin/sh
> -# ensure that dd's oflag=direct works
> +# ensure that dd's iflag=direct and oflag=direct work
>  
>  # Copyright (C) 2009-2017 Free Software Foundation, Inc.
>  
> @@ -29,7 +29,7 @@ truncate -s 8193 p1 || framework_failure_
>  
>  for i in short m1 p1; do
>    rm -f out
> -  dd if=$i oflag=direct of=out || fail=1
> +  dd if=$i iflag=direct oflag=direct of=out || fail=1
>  done
>  
>  Exit $fail
> 

This will need adjusting.
It works on ext4,
and on newer 4.10.6-200.fc25.x86_64) xfs, where it's not actually needed as 
EINVAL is not returned
but on older (centos7) xfs, even the initial read() will return EINVAL,
so we'd have to disable O_DIRECT and retry to support that, which I'm not sure 
about.
I'll do a bit more testing...

cheers,
Pádraig