Re: [PATCH 1/3] vhost-user-blk-test: fix Coverity open(2) false positives

[Peter Maydell]

On Wed, 26 May 2021 at 10:16, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Coverity checks that the file descriptor return value of open(2) is
> checked and used. Normally this is helpful in identifying real bugs but
> vhost-user-blk-test opens /dev/null as stdin and stdout after fork.
>
> In this case we don't need to look at the return value because these
> open(2) calls cannot fail in any reasonable environment. We already know
> their return values ahead of time since we closed stdin and stdout
> previously. open(2) is guaranteed to pick the lowest available fd
> number.
>
> Silence Coverity by introducing code that checks what we already know to
> be true.

> diff --git a/tests/qtest/vhost-user-blk-test.c 
> b/tests/qtest/vhost-user-blk-test.c
> index 8796c74ca4..581e283a03 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -910,14 +910,18 @@ static void start_vhost_user_blk(GString *cmd_line, int 
> vus_instances,
>                     storage_daemon_command->str);
>      pid_t pid = fork();
>      if (pid == 0) {
> +        int fd;
> +
>          /*
>           * Close standard file descriptors so tap-driver.pl pipe detects when
>           * our parent terminates.
>           */
>          close(0);
> +        fd = open("/dev/null", O_RDONLY);
> +        g_assert_cmpint(fd, ==, 0);
>          close(1);
> -        open("/dev/null", O_RDONLY);
> -        open("/dev/null", O_WRONLY);
> +        fd = open("/dev/null", O_WRONLY);
> +        assert(fd == 1);


Why use a different assert type for the two asserts?

thanks
-- PMM