On 6/1/21 5:57 PM, Stefan Hajnoczi 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.
>
> ** CID 1453270: Resource leaks (RESOURCE_LEAK)
> /qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
>
> ________________________________________________________________________________________________________
> *** CID 1453270: Resource leaks (RESOURCE_LEAK)
> /qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
> 914 * Close standard file descriptors so tap-driver.pl pipe
> detects when
> 915 * our parent terminates.
> 916 */
> 917 close(0);
> 918 close(1);
> 919 open("/dev/null", O_RDONLY);
>>>> CID 1453270: Resource leaks (RESOURCE_LEAK)
>>>> Ignoring handle opened by "open("/dev/null", 1)" leaks it.
> 920 open("/dev/null", O_WRONLY);
> 921
> 922 execlp("/bin/sh", "sh", "-c", storage_daemon_command->str,
> NULL);
> 923 exit(1);
> 924 }
> 925 g_string_free(storage_daemon_command, true);
>
> ** CID 1453269: Error handling issues (NEGATIVE_RETURNS)
> /qemu/tests/qtest/vhost-user-blk-test.c: 829 in create_listen_socket()
>
> ________________________________________________________________________________________________________
> *** CID 1453269: Error handling issues (NEGATIVE_RETURNS)
> /qemu/tests/qtest/vhost-user-blk-test.c: 829 in create_listen_socket()
> 823 char *path;
> 824
> 825 /* No race because our pid makes the path unique */
> 826 path = g_strdup_printf("/tmp/qtest-%d-sock.XXXXXX", getpid());
> 827 tmp_fd = mkstemp(path);
> 828 g_assert_cmpint(tmp_fd, >=, 0);
>>>> CID 1453269: Error handling issues (NEGATIVE_RETURNS)
>>>> "tmp_fd" is passed to a parameter that cannot be negative.
> 829 close(tmp_fd);
> 830 unlink(path);
> 831
> 832 *fd = qtest_socket_server(path);
> 833 g_test_queue_destroy(destroy_file, path);
> 834 return path;
>
> ** CID 1453268: (CHECKED_RETURN)
> /qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
> /qemu/tests/qtest/vhost-user-blk-test.c: 919 in start_vhost_user_blk()
>
> ________________________________________________________________________________________________________
> *** CID 1453268: (CHECKED_RETURN)
> /qemu/tests/qtest/vhost-user-blk-test.c: 920 in start_vhost_user_blk()
> 914 * Close standard file descriptors so tap-driver.pl pipe
> detects when
> 915 * our parent terminates.
> 916 */
> 917 close(0);
> 918 close(1);
> 919 open("/dev/null", O_RDONLY);
>>>> CID 1453268: (CHECKED_RETURN)
>>>> Calling "open("/dev/null", 1)" without checking return value. This
>>>> library function may fail and return an error code. [Note: The source code
>>>> implementation of the function has been overridden by a builtin model.]
> 920 open("/dev/null", O_WRONLY);
> 921
> 922 execlp("/bin/sh", "sh", "-c", storage_daemon_command->str,
> NULL);
> 923 exit(1);
> 924 }
> 925 g_string_free(storage_daemon_command, true);
> /qemu/tests/qtest/vhost-user-blk-test.c: 919 in start_vhost_user_blk()
> 913 /*
> 914 * Close standard file descriptors so tap-driver.pl pipe
> detects when
> 915 * our parent terminates.
> 916 */
> 917 close(0);
> 918 close(1);
>>>> CID 1453268: (CHECKED_RETURN)
>>>> Calling "open("/dev/null", 0)" without checking return value. This
>>>> library function may fail and return an error code. [Note: The source code
>>>> implementation of the function has been overridden by a builtin model.]
> 919 open("/dev/null", O_RDONLY);
> 920 open("/dev/null", O_WRONLY);
> 921
> 922 execlp("/bin/sh", "sh", "-c", storage_daemon_command->str,
> NULL);
> 923 exit(1);
> 924 }
>
> Signed-off-by: Stefan Hajnoczi <[email protected]>
> ---
> tests/qtest/vhost-user-blk-test.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
