[LTP] [PATCH v2 1/2] Add shutdown01 test

[Andrea Cervesato]

Hi Petr,

I agree with the proposed changes. Thanks, feel free to merge.

Acked-by: Andrea Cervesato <andrea.cervesato@suse.com>

On 6/10/24 15:13, Petr Vorel wrote:
> Hi Andrea,
>
> TL;DR: generally LGTM, I have few minor details, which I offer to fix before
> merge (diff below).
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
>> +++ b/testcases/kernel/syscalls/shutdown/shutdown01.c
> ...
>> +static struct tcase {
>> +	int shutdown_op;
>> +	int recv_flag;
>> +	int recv_err;
>> +	int send_flag;
>> +	int send_err;
>> +	char *flag_str;
>> +} tcases[] = {
>> +	{SHUT_RD, 0, 0, 0, 0, "SHUT_RD"},
>> +	{SHUT_WR, MSG_DONTWAIT, EWOULDBLOCK, MSG_NOSIGNAL, EPIPE, "SHUT_WR"},
>> +	{SHUT_RDWR, 0, 0, MSG_NOSIGNAL, EPIPE, "SHUT_RDWR"}
> Would you mind to use
> 1) designated initializers (e.g. .recv_flag = MSG_DONTWAIT,)
> 2) stringify macro
>
> IMHO it helps readability, that's why it's often used.
>
> #define OP_DESC(x) .shutdown_op = x, .desc = #x
> static struct tcase {
> 	int shutdown_op;
> 	int recv_flag;
> 	int recv_err;
> 	int send_flag;
> 	int send_err;
> 	char *desc;
> } tcases[] = {
> 	{OP_DESC(SHUT_RD)},
> 	{OP_DESC(SHUT_WR), .recv_flag = MSG_DONTWAIT, .recv_err = EWOULDBLOCK,
> 		.send_flag = MSG_NOSIGNAL, .send_err = EPIPE},
> 	{OP_DESC(SHUT_RDWR), .send_flag = MSG_NOSIGNAL, .send_err = EPIPE}
> };
>
>> +};
>> +
>> +static struct sockaddr_un *sock_addr;
>> +
>> +static void run_server(void)
>> +{
>> +	int server_sock;
>> +
>> +	server_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
>> +
>> +	SAFE_BIND(server_sock,
>> +		(struct sockaddr *)sock_addr,
>> +		sizeof(struct sockaddr_un));
>> +	SAFE_LISTEN(server_sock, 10);
>> +
>> +	tst_res(TINFO, "Running server on socket file");
>> +
>> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
>> +
>> +	SAFE_CLOSE(server_sock);
>> +	SAFE_UNLINK(SOCKETFILE);
>> +}
>> +
>> +static int start_test(void)
>> +{
>> +	int client_sock;
>> +
>> +	if (!SAFE_FORK()) {
>> +		run_server();
>> +		_exit(0);
>> +	}
>> +
>> +	TST_CHECKPOINT_WAIT(0);
>> +
>> +	tst_res(TINFO, "Connecting to the server");
>> +
>> +	client_sock = SAFE_SOCKET(sock_addr->sun_family, SOCK_STREAM, 0);
>> +	SAFE_CONNECT(client_sock,
>> +		(struct sockaddr *)sock_addr,
>> +		sizeof(struct sockaddr_un));
>> +
>> +	return client_sock;
>> +}
>> +
>> +static void run(unsigned int n)
>> +{
>> +	struct tcase *tc = &tcases[n];
>> +	int client_sock;
>> +	char buff[MSGSIZE] = {0};
>> +
>> +	client_sock = start_test();
>> +
>> +	tst_res(TINFO, "Testing %s flag", tc->flag_str);
>> +
>> +	TST_EXP_PASS(shutdown(client_sock, tc->shutdown_op));
>> +
>> +	if (!tc->recv_err)
>> +		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
>> +	else
>> +		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
> very nit:
>
> 	if (tc->recv_err)
> 		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
> 	else
> 		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
>
> If you agree, I can merge with following changes.
>
> Kind regards,
> Petr
>
> diff --git testcases/kernel/syscalls/shutdown/shutdown01.c testcases/kernel/syscalls/shutdown/shutdown01.c
> index ba3853d9c..8e58f23e6 100644
> --- testcases/kernel/syscalls/shutdown/shutdown01.c
> +++ testcases/kernel/syscalls/shutdown/shutdown01.c
> @@ -19,17 +19,19 @@
>   #define MSGSIZE 4
>   #define SOCKETFILE "socket"
>   
> +#define OP_DESC(x) .shutdown_op = x, .desc = #x
>   static struct tcase {
>   	int shutdown_op;
>   	int recv_flag;
>   	int recv_err;
>   	int send_flag;
>   	int send_err;
> -	char *flag_str;
> +	char *desc;
>   } tcases[] = {
> -	{SHUT_RD, 0, 0, 0, 0, "SHUT_RD"},
> -	{SHUT_WR, MSG_DONTWAIT, EWOULDBLOCK, MSG_NOSIGNAL, EPIPE, "SHUT_WR"},
> -	{SHUT_RDWR, 0, 0, MSG_NOSIGNAL, EPIPE, "SHUT_RDWR"}
> +	{OP_DESC(SHUT_RD)},
> +	{OP_DESC(SHUT_WR), .recv_flag = MSG_DONTWAIT, .recv_err = EWOULDBLOCK,
> +		.send_flag = MSG_NOSIGNAL, .send_err = EPIPE},
> +	{OP_DESC(SHUT_RDWR), .send_flag = MSG_NOSIGNAL, .send_err = EPIPE}
>   };
>   
>   static struct sockaddr_un *sock_addr;
> @@ -82,23 +84,22 @@ static void run(unsigned int n)
>   
>   	client_sock = start_test();
>   
> -	tst_res(TINFO, "Testing %s flag", tc->flag_str);
> +	tst_res(TINFO, "Testing %s flag", tc->desc);
>   
>   	TST_EXP_PASS(shutdown(client_sock, tc->shutdown_op));
>   
> -	if (!tc->recv_err)
> -		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
> -	else
> +	if (tc->recv_err)
>   		TST_EXP_FAIL(recv(client_sock, buff, MSGSIZE, tc->recv_flag), tc->recv_err);
> -
> -	if (!tc->send_err)
> -		SAFE_SEND(MSGSIZE, client_sock, buff, MSGSIZE, tc->send_flag);
>   	else
> +		SAFE_RECV(0, client_sock, buff, MSGSIZE, tc->recv_flag);
> +
> +	if (tc->send_err)
>   		TST_EXP_FAIL(send(client_sock, buff, MSGSIZE, tc->send_flag), tc->send_err);
> +	else
> +		SAFE_SEND(MSGSIZE, client_sock, buff, MSGSIZE, tc->send_flag);
>   
>   	SAFE_CLOSE(client_sock);
>   	TST_CHECKPOINT_WAKE(0);
> -
>   }
>   
>   static void setup(void)

Andrea