Re: [Qemu-devel] [PATCH] fix fcntl support in linux-user.

[Riku Voipio <riku.voipio@iki.fi>]

On Sun, Apr 19, 2009 at 10:45:39PM +0200, Arnaud Patard wrote:
> 
> The current code in do_fcntl is passing the target command as a host
> command. This is introducing 2 problems:
> 
> - When building with "-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE", we
>   may end up passing commands like F_GETFL instead of F_GETFL64. This is
>   likely to return an error.
> 
> - The F_*64 constants doesn't always have the same value between host
>   and target.

This appears to be the case for some other fcntl constants as well. Howabout
implementing this as a target_to_host_fcntl_cmd() mapping function instead?

> Without this patch locking is not working (My test-case was pwck with arm as
> target and mips as host).

Could you also add the MIPS target definition for F_*64 so that mips on arm
would get fixed too?

> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> ---

> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 2d51d6b..2d876c1 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3126,7 +3141,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          fl.l_len = tswapl(target_fl->l_len);
>          fl.l_pid = tswapl(target_fl->l_pid);
>          unlock_user_struct(target_fl, arg, 0);
> -        ret = get_errno(fcntl(fd, cmd, &fl));
> +        ret = get_errno(fcntl(fd, F_GETLK, &fl));
>          if (ret == 0) {
>              if (!lock_user_struct(VERIFY_WRITE, target_fl, arg, 0))
>                  return -TARGET_EFAULT;
> @@ -3149,7 +3164,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          fl.l_len = tswapl(target_fl->l_len);
>          fl.l_pid = tswapl(target_fl->l_pid);
>          unlock_user_struct(target_fl, arg, 0);
> -        ret = get_errno(fcntl(fd, cmd, &fl));
> +        ret = get_errno(fcntl(fd, F_SETLK+(cmd-TARGET_F_SETLK), &fl));
>          break;
>  
>      case TARGET_F_GETLK64:
> @@ -3161,7 +3176,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          fl64.l_len = tswapl(target_fl64->l_len);
>          fl64.l_pid = tswap16(target_fl64->l_pid);
>          unlock_user_struct(target_fl64, arg, 0);
> -        ret = get_errno(fcntl(fd, cmd >> 1, &fl64));
> +        ret = get_errno(fcntl(fd, F_GETLK64, &fl64));
>          if (ret == 0) {
>              if (!lock_user_struct(VERIFY_WRITE, target_fl64, arg, 0))
>                  return -TARGET_EFAULT;
> @@ -3183,7 +3198,7 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
>          fl64.l_len = tswapl(target_fl64->l_len);
>          fl64.l_pid = tswap16(target_fl64->l_pid);
>          unlock_user_struct(target_fl64, arg, 0);
> -        ret = get_errno(fcntl(fd, cmd >> 1, &fl64));
> +        ret = get_errno(fcntl(fd, F_SETLK64+(cmd-TARGET_F_SETLK64), &fl64));
>          break;
>  
>      case F_GETFL:
> @@ -6022,7 +6039,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              ret = get_errno(fcntl(arg1, cmd, &fl));
>  	    break;
>          default:
> -            ret = do_fcntl(arg1, cmd, arg3);
> +            ret = do_fcntl(arg1, arg2, arg3);
>              break;
>          }
>  	break;