Re: [LTP] [PATCH v1 1/2] fcntl{34, 36}: Only use fcntl64 with 32bit abi

[Petr Vorel <pvorel@suse.cz>]

Hi Edward,

> Fixes: 7643115aaacb ("fcntl{34,36}: Always use 64-bit flock struct to avoid EINVAL")

> On Android arm64, tst_kernel_bits is disregarding the abi, so compiling
What exactly do you mean by "disregarding the abi"? Why is aarch64 different?

> with the 32bit abi is calling the fcntl syscall instead of fcntl64.  The
> fcntl syscall is not compatible with the flock64 struct being passed
> (this doesn't seem to be the case with x86_64, only with arm64).

> This changes it to only use the fcntl64 compat syscall with the 32bit
> abi.

> Signed-off-by: Edward Liaw <edliaw@google.com>
> ---
>  testcases/kernel/syscalls/fcntl/fcntl_common.h | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

> diff --git a/testcases/kernel/syscalls/fcntl/fcntl_common.h b/testcases/kernel/syscalls/fcntl/fcntl_common.h
> index 5c130a784..485a31367 100644
> --- a/testcases/kernel/syscalls/fcntl/fcntl_common.h
> +++ b/testcases/kernel/syscalls/fcntl/fcntl_common.h
> @@ -10,6 +10,11 @@
>  #include "lapi/abisize.h"
>  #include "lapi/fcntl.h"

> +#if defined(TST_ABI64)
> +#define FCNTL_COMPAT(fd, cmd, flock) \
> +	SAFE_FCNTL(fd, cmd, flock)
> +
> +#else
>  struct my_flock64 {
>  	int16_t l_type;
>  	int16_t l_whence;
> @@ -43,8 +48,8 @@ static inline int fcntl_compat(const char *file, const int line, const char *cmd
>  		.l_len = lck->l_len,
>  		.l_pid = lck->l_pid,
>  	};
> -	const int sysno = tst_kernel_bits() > 32 ? __NR_fcntl : __NR_fcntl64;
> -	const int ret = tst_syscall(sysno, fd, cmd, &l64);
> +
> +	const int ret = tst_syscall(__NR_fcntl64, fd, cmd, &l64);

>  	lck->l_type = l64.l_type;
>  	lck->l_whence = l64.l_whence;
> @@ -57,7 +62,7 @@ static inline int fcntl_compat(const char *file, const int line, const char *cmd

>  	tst_brk_(file, line, TBROK | TERRNO,
>  		 "%s(%d, %s, { %d, %d, %"PRId64", %"PRId64", %d })",
> -		 tst_kernel_bits() > 32 ? "fcntl" : "fcntl64",
> +		 "fcntl64",

Once we removed tst_kernel_bits(), there is no need to pass "fcntl64" as %s,
thus it should be:

		 "fcntl64(%d, %s, { %d, %d, %"PRId64", %"PRId64", %d })",
		 fd, cmd_name, l64.l_type, l64.l_whence, l64.l_start, l64.l_len,
		 l64.l_pid);

Otherwise LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

>  		 fd,
>  		 cmd_name,
>  		 l64.l_type, l64.l_whence, l64.l_start, l64.l_len, l64.l_pid);
> @@ -67,5 +72,6 @@ static inline int fcntl_compat(const char *file, const int line, const char *cmd

>  #define FCNTL_COMPAT(fd, cmd, flock) \
>  	fcntl_compat(__FILE__, __LINE__, #cmd, fd, cmd, flock)
> +#endif

>  #endif /* FCNTL_COMMON_H__ */

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp