Re: [Qemu-devel] [PATCH v2 4/7] linux-user: Fix fcnt

[Peter Maydell <peter.maydell@linaro.org>]

On 24 November 2016 at 16:08, Lena Djokic <Lena.Djokic@rt-rk.com> wrote:

Making the subject line "linux-user: fix F_GETSIG and F_SETSIG fcntls"
would be a bit more precise about what we're fixing here and I think
that will be helpful for people looking back in the git log later.

> F_GETSIG and F_SETSIG were implemented with default behaviour which
> simply passes given arguments to fcntl syscall, but since those
> arguments are signals used for communication between taget and

typo: "target"

> host we need conversion which is done by using host_to_target signal
> and taget_to_host_signal functions.

"target"

>
> Signed-off-by: Lena Djokic <Lena.Djokic@rt-rk.com>
> ---
>  linux-user/syscall.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 41873ca..1b59a71 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6544,14 +6544,18 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>
>      case TARGET_F_SETOWN:
>      case TARGET_F_GETOWN:
> -    case TARGET_F_SETSIG:
> -    case TARGET_F_GETSIG:
>      case TARGET_F_SETLEASE:
>      case TARGET_F_GETLEASE:
>      case TARGET_F_SETPIPE_SZ:
>      case TARGET_F_GETPIPE_SZ:
>          ret = get_errno(safe_fcntl(fd, host_cmd, arg));
>          break;
> +    case TARGET_F_GETSIG:
> +        ret = host_to_target_signal(get_errno(fcntl(fd, host_cmd, arg)));
> +        break;
> +    case TARGET_F_SETSIG:
> +        ret = get_errno(fcntl(fd, host_cmd, target_to_host_signal(arg)));
> +        break;

This is basically right, but I suggest some minor changes:
 * put these special cases above the long list of 'just call fcntl'
   cases, not below it
 * these should both call safe_fcntl(), not fcntl()
 * don't call host_to_target_signal() unless we know the call
   succeeded (compare the code used for TARGET_F_GETLK, TARGET_F_GETFL, etc)

>
>      default:
>          ret = get_errno(safe_fcntl(fd, cmd, arg));

thanks
-- PMM