Re: [PATCH liburing] Fix incorrect close in test for multishot accept

[Hao Xu <hao.xu@linux.dev>]

On 6/17/22 00:22, Donald Hunter wrote:
> This fixes a bug in accept_conn handling in the accept tests that caused it
> to incorrectly skip the multishot tests and also lose the warning message
> to a closed stdout. This can be seen in the strace output below.
> 
> close(1)                                = 0
> io_uring_setup(32, { ...
> ...
> write(1, "Fixed Multishot Accept not suppo"..., 47) = -1 EINVAL
> 
> Unfortunately this exposes a a bug with gcc -O2 where multishot_mask logic
> gets optimized incorrectly and "Fixed Multishot Accept misses events" is
> wrongly reported. I am investigating this separately.
> 

Super thanks, Donald. you are right, we skipped the fixed multishot test
by mistake, the exposed issue after your fix is caused by
multishot_mask |= (1 << (s_fd[i] - 1))
which should be
multishot_mask |= (1U << s_fd[i])

Would you mind me to take this one to my patch series which is to fix
this and do some cleaning?

> Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
> ---
>   test/accept.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/test/accept.c b/test/accept.c
> index 7bc6226..fb87a1d 100644
> --- a/test/accept.c
> +++ b/test/accept.c
> @@ -103,7 +103,7 @@ static void queue_accept_conn(struct io_uring *ring, int fd,
>   	}
>   }
>   
> -static int accept_conn(struct io_uring *ring, int fixed_idx)
> +static int accept_conn(struct io_uring *ring, int fixed_idx, bool multishot)
>   {
>   	struct io_uring_cqe *cqe;
>   	int ret;
> @@ -115,8 +115,10 @@ static int accept_conn(struct io_uring *ring, int fixed_idx)
>   
>   	if (fixed_idx >= 0) {
>   		if (ret > 0) {
> -			close(ret);
> -			return -EINVAL;
> +			if (!multishot) {
> +				close(ret);
> +				return -EINVAL;
> +			}
>   		} else if (!ret) {
>   			ret = fixed_idx;
>   		}
> @@ -208,7 +210,7 @@ static int test_loop(struct io_uring *ring,
>   		queue_accept_conn(ring, recv_s0, args);
>   
>   	for (i = 0; i < MAX_FDS; i++) {
> -		s_fd[i] = accept_conn(ring, args.fixed ? 0 : -1);
> +		s_fd[i] = accept_conn(ring, args.fixed ? 0 : -1, multishot);
>   		if (s_fd[i] == -EINVAL) {
>   			if (args.accept_should_error)
>   				goto out;