[PATCH] net/vhost-vdpa.c: Fix clang compilation failure

[PATCH] net/vhost-vdpa.c: Fix clang compilation failure

[Peter Maydell]

Commit 8801ccd0500437 introduced a compilation failure with clang
version 10.0.0-4ubuntu1:

../../net/vhost-vdpa.c:654:16: error: variable 'vdpa_device_fd' is
used uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
    } else if (opts->has_vhostfd) {
               ^~~~~~~~~~~~~~~~~
../../net/vhost-vdpa.c:662:33: note: uninitialized use occurs here
    r = vhost_vdpa_get_features(vdpa_device_fd, &features, errp);
                                ^~~~~~~~~~~~~~
../../net/vhost-vdpa.c:654:12: note: remove the 'if' if its condition
is always true
    } else if (opts->has_vhostfd) {
           ^~~~~~~~~~~~~~~~~~~~~~~
../../net/vhost-vdpa.c:629:23: note: initialize the variable
'vdpa_device_fd' to silence this warning
    int vdpa_device_fd;
                      ^
                       = 0
1 error generated.

It's a false positive -- the compiler doesn't manage to figure out
that the error checks further up mean that there's no code path where
vdpa_device_fd isn't initialized.  Put another way, the problem is
that we check "if (opts->has_vhostfd)" when in fact that condition
must always be true.  A cleverer static analyser would probably warn
that we were checking an always-true condition.

Fix the compilation failure by removing the unnecessary if().

Fixes: 8801ccd0500437 ("vhost-vdpa: allow passing opened vhostfd to vhost-vdpa")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Stefan, do you want to apply this directly as a build fix?
---
 net/vhost-vdpa.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 854ebd61ae6..e370ecb8ebd 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -651,7 +651,8 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
         if (vdpa_device_fd == -1) {
             return -errno;
         }
-    } else if (opts->has_vhostfd) {
+    } else {
+        /* has_vhostfd */
         vdpa_device_fd = monitor_fd_param(monitor_cur(), opts->vhostfd, errp);
         if (vdpa_device_fd == -1) {
             error_prepend(errp, "vhost-vdpa: unable to parse vhostfd: ");
-- 
2.25.1

Re: [PATCH] net/vhost-vdpa.c: Fix clang compilation failure

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]

On Mon, Oct 31, 2022 at 01:29:01PM +0000, Peter Maydell wrote:
> Commit 8801ccd0500437 introduced a compilation failure with clang
> version 10.0.0-4ubuntu1:
> 
> ../../net/vhost-vdpa.c:654:16: error: variable 'vdpa_device_fd' is
> used uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
>     } else if (opts->has_vhostfd) {
>                ^~~~~~~~~~~~~~~~~
> ../../net/vhost-vdpa.c:662:33: note: uninitialized use occurs here
>     r = vhost_vdpa_get_features(vdpa_device_fd, &features, errp);
>                                 ^~~~~~~~~~~~~~
> ../../net/vhost-vdpa.c:654:12: note: remove the 'if' if its condition
> is always true
>     } else if (opts->has_vhostfd) {
>            ^~~~~~~~~~~~~~~~~~~~~~~
> ../../net/vhost-vdpa.c:629:23: note: initialize the variable
> 'vdpa_device_fd' to silence this warning
>     int vdpa_device_fd;
>                       ^
>                        = 0
> 1 error generated.
> 
> It's a false positive -- the compiler doesn't manage to figure out
> that the error checks further up mean that there's no code path where
> vdpa_device_fd isn't initialized.  Put another way, the problem is
> that we check "if (opts->has_vhostfd)" when in fact that condition
> must always be true.  A cleverer static analyser would probably warn
> that we were checking an always-true condition.
> 
> Fix the compilation failure by removing the unnecessary if().
> 
> Fixes: 8801ccd0500437 ("vhost-vdpa: allow passing opened vhostfd to vhost-vdpa")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Stefan, do you want to apply this directly as a build fix?
> ---
>  net/vhost-vdpa.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Thanks, applied to the staging tree:
https://gitlab.com/qemu-project/qemu/commits/staging

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]