[PATCH] posix_fadvise() *returns* error codes but does not set errno

Corinna Vinschen corinna-cygwin@cygwin.com
Thu Nov 2 14:58:00 GMT 2017


Hi Eric,

On Nov  2 14:26, Erik M. Bray wrote:
> Also updates the fhandler_*::fadvise implementations to adhere to the same
> semantics.

Good catch.  I have just some style nits.

> ---
>  winsup/cygwin/fhandler.cc           |  3 +--
>  winsup/cygwin/fhandler_disk_file.cc | 16 ++++++----------
>  winsup/cygwin/pipe.cc               |  3 +--
>  winsup/cygwin/syscalls.cc           |  2 +-
>  4 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
> index d719b7c..858c1fd 100644
> --- a/winsup/cygwin/fhandler.cc
> +++ b/winsup/cygwin/fhandler.cc
> @@ -1764,8 +1764,7 @@ fhandler_base::fsetxattr (const char *name, const void *value, size_t size,
>  int
>  fhandler_base::fadvise (off_t offset, off_t length, int advice)
>  {
> -  set_errno (EINVAL);
> -  return -1;
> +  return EINVAL;
>  }
>  
>  int
> diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
> index 2144a4c..f46e355 100644
> --- a/winsup/cygwin/fhandler_disk_file.cc
> +++ b/winsup/cygwin/fhandler_disk_file.cc
> @@ -1076,8 +1076,7 @@ fhandler_disk_file::fadvise (off_t offset, off_t length, int advice)
>  {
>    if (advice < POSIX_FADV_NORMAL || advice > POSIX_FADV_NOREUSE)
>      {
> -      set_errno (EINVAL);
> -      return -1;
> +      return EINVAL;
>      }

Please remove the braces for a one-line block.

>  
>    /* Windows only supports advice flags for the whole file.  We're using
> @@ -1097,21 +1096,18 @@ fhandler_disk_file::fadvise (off_t offset, off_t length, int advice)
>    NTSTATUS status = NtQueryInformationFile (get_handle (), &io,
>  					    &fmi, sizeof fmi,
>  					    FileModeInformation);
> -  if (!NT_SUCCESS (status))
> -    __seterrno_from_nt_status (status);
> -  else
> +  if (NT_SUCCESS (status))
>      {
>        fmi.Mode &= ~FILE_SEQUENTIAL_ONLY;
>        if (advice == POSIX_FADV_SEQUENTIAL)
> -	fmi.Mode |= FILE_SEQUENTIAL_ONLY;
> +        fmi.Mode |= FILE_SEQUENTIAL_ONLY;

You changed indentation here for no apparent reason (TABs vs spaces).

>        status = NtSetInformationFile (get_handle (), &io, &fmi, sizeof fmi,
> -				     FileModeInformation);
> +                                     FileModeInformation);
>        if (NT_SUCCESS (status))
> -	return 0;
> -      __seterrno_from_nt_status (status);
> +	    return 0;
>      }

Ditto and ditto.

> -  return -1;
> +  return geterrno_from_nt_status (status);
>  }
>  
>  int
> diff --git a/winsup/cygwin/pipe.cc b/winsup/cygwin/pipe.cc
> index 79b7966..8738d34 100644
> --- a/winsup/cygwin/pipe.cc
> +++ b/winsup/cygwin/pipe.cc
> @@ -165,8 +165,7 @@ fhandler_pipe::lseek (off_t offset, int whence)
>  int
>  fhandler_pipe::fadvise (off_t offset, off_t length, int advice)
>  {
> -  set_errno (ESPIPE);
> -  return -1;
> +  return ESPIPE;
>  }
>  
>  int
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index caa3a77..d0d735b 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -2937,7 +2937,7 @@ posix_fadvise (int fd, off_t offset, off_t len, int advice)
>    if (cfd >= 0)
>      res = cfd->fadvise (offset, len, advice);
>    else
> -    set_errno (EBADF);
> +    res = EBADF;
>    syscall_printf ("%R = posix_fadvice(%d, %D, %D, %d)",
>  		  res, fd, offset, len, advice);
>    return res;
> -- 
> 2.8.3

Other than that, looks good.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20171102/8049d24f/attachment.sig>


More information about the Cygwin-patches mailing list