[GSOC][PATCH v2] tests/fcntl.c: add test for struct f_owner_ex

[Dmitry V. Levin]

On Tue, Mar 27, 2018 at 01:20:14AM +0800, Zhibin Li wrote:
> Signed-off-by: Zhibin Li <08826794brmt at gmail.com>
> ---
>  tests/fcntl.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/tests/fcntl.c b/tests/fcntl.c
> index 4f62ca2a..8c74ed22 100644
> --- a/tests/fcntl.c
> +++ b/tests/fcntl.c
> @@ -36,6 +36,9 @@
>  # include "struct_flock.c"
>  
>  # define TEST_FLOCK64_EINVAL(cmd) test_flock64_einval(cmd, #cmd)
> +# define TEST_F_OWNER_EX_ESRCH(cmd) test_f_owner_ex_esrch(cmd, #cmd)
> +
> +typedef struct f_owner_ex struct_kernel_f_owner_ex;

The new typedef is used only once, do you really need it now?

>  static void
>  test_flock64_einval(const int cmd, const char *name)
> @@ -69,12 +72,72 @@ test_flock64(void)
>  #endif
>  }
>  
> +static int
> +test_f_owner_ex_type_pid(const int cmd, const char *const cmd_name,
> +				const int type, const char *const type_name,
> +				const pid_t pid)
> +{
> +	struct_kernel_f_owner_ex fo = { .type = type, .pid = pid };
> +
> +	long rc = invoke_test_syscall(cmd, &fo);
> +	printf("%s(0, %s, {type=%s, pid=%d}) = %s\n",
> +		TEST_SYSCALL_STR, cmd_name, type_name, pid, sprintrc(rc));
> +	if (rc)
> +		return 1;
> +	return 0;

There is a simple way to write this:
	return !!rc;

Alternatively, you can change the return type from "int" to "long"
and just return rc.

> +}
> +
> +static void
> +test_f_owner_ex_esrch(const int cmd, const char *const cmd_name)
> +{
> +	test_f_owner_ex_type_pid(cmd, cmd_name,
> +					ARG_STR(F_OWNER_TID), 1234567890);
> +	test_f_owner_ex_type_pid(cmd, cmd_name,
> +					ARG_STR(F_OWNER_PID), 1234567890);
> +	test_f_owner_ex_type_pid(cmd, cmd_name,
> +					ARG_STR(F_OWNER_PGRP), -1234567890);

Something went wrong with the indentation in this function.

> +}
> +
> +static void
> +test_f_owner_ex(void)
> +{
> +	TEST_F_OWNER_EX_ESRCH(F_SETOWN_EX);

Looks like this could be written without TEST_F_OWNER_EX_ESRCH macro:

	test_f_owner_ex_esrch(ARG_STR(F_SETOWN_EX));

> +
> +	long rc;
> +
> +	rc = test_f_owner_ex_type_pid(ARG_STR(F_SETOWN_EX),
> +					ARG_STR(F_OWNER_TID), 20);
> +	if (rc)
> +		return;
> +
> +	test_f_owner_ex_type_pid(ARG_STR(F_GETOWN_EX),
> +					ARG_STR(F_OWNER_TID), 20);
> +
> +	rc = test_f_owner_ex_type_pid(ARG_STR(F_SETOWN_EX),
> +					ARG_STR(F_OWNER_PID), 30);
> +	if (rc)
> +		return;
> +
> +	test_f_owner_ex_type_pid(ARG_STR(F_GETOWN_EX),
> +					ARG_STR(F_OWNER_PID), 30);
> +
> +	rc = test_f_owner_ex_type_pid(ARG_STR(F_SETOWN_EX),
> +					ARG_STR(F_OWNER_PGRP), 40);
> +	if (rc)
> +		return;
> +
> +	test_f_owner_ex_type_pid(ARG_STR(F_GETOWN_EX),
> +					ARG_STR(F_OWNER_PGRP), 40);

The same F_SETOWN_EX/check/F_GETOWN_EX pattern is repeated thrice here,
wouldn't it be better if it was moved to a separate function?


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180326/37bc8ee1/attachment.bin>