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

[Stefan Hajnoczi]

On Sun, May 30, 2021 at 08:05:49PM +0100, Peter Maydell wrote:
> 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 for pointing this out. I will send a v2 that consistently uses
g_assert_cmpint().

Stefan

signature.asc
Description: PGP signature