[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/3] vhost-user-blk-test: fix Coverity open(2) false positive
From: |
Peter Maydell |
Subject: |
Re: [PATCH 1/3] vhost-user-blk-test: fix Coverity open(2) false positives |
Date: |
Sun, 30 May 2021 20:05:49 +0100 |
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