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
signature.asc
Description: PGP signature
