[PATCH 0/4] assorted gcc 10/fedora32 compile warning fixes

[PATCH 0/4] assorted gcc 10/fedora32 compile warning fixes

[Christian Borntraeger]

I might be wrong and some of these warnings could be correct, so some
review from the subject matter experts would be good.

Christian Borntraeger (4):
  vmdk: fix maybe uninitialized warnings
  nbd: silence maybe-uninitialized warnings
  qemu-io-cmds: avoid gcc 10 warning
  virtiofsd: avoid false positive compiler warning

 block/vmdk.c                     | 8 ++++----
 nbd/server.c                     | 2 +-
 qemu-io-cmds.c                   | 4 +++-
 tools/virtiofsd/passthrough_ll.c | 2 +-
 4 files changed, 9 insertions(+), 7 deletions(-)

-- 
2.26.2

[PATCH 1/4] vmdk: fix maybe uninitialized warnings

[Christian Borntraeger]

Fedora 32 gcc 10 seems to give false positives:

Compiling C object libblock.fa.p/block_vmdk.c.o
../block/vmdk.c: In function ‘vmdk_parse_extents’:
../block/vmdk.c:587:5: error: ‘extent’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  587 |     g_free(extent->l1_table);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
../block/vmdk.c:754:17: note: ‘extent’ was declared here
  754 |     VmdkExtent *extent;
      |                 ^~~~~~
../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  620 |     ret = vmdk_init_tables(bs, extent, errp);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../block/vmdk.c:598:17: note: ‘extent’ was declared here
  598 |     VmdkExtent *extent;
      |                 ^~~~~~
../block/vmdk.c:1178:39: error: ‘extent’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 1178 |             extent->flat_start_offset = flat_offset << 9;
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
../block/vmdk.c: In function ‘vmdk_open_vmdk4’:
../block/vmdk.c:581:22: error: ‘extent’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  581 |     extent->l2_cache =
      |     ~~~~~~~~~~~~~~~~~^
  582 |         g_malloc(extent->entry_size * extent->l2_size * L2_CACHE_SIZE);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../block/vmdk.c:872:17: note: ‘extent’ was declared here
  872 |     VmdkExtent *extent;
      |                 ^~~~~~
../block/vmdk.c: In function ‘vmdk_open’:
../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  620 |     ret = vmdk_init_tables(bs, extent, errp);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../block/vmdk.c:598:17: note: ‘extent’ was declared here
  598 |     VmdkExtent *extent;
      |                 ^~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile.ninja:884: libblock.fa.p/block_vmdk.c.o] Error 1

fix them by assigning a default value.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 block/vmdk.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 8ec62c7ab798..a00dc00eb47a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -595,7 +595,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
     int ret;
     uint32_t magic;
     VMDK3Header header;
-    VmdkExtent *extent;
+    VmdkExtent *extent = NULL;
 
     ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
     if (ret < 0) {
@@ -751,7 +751,7 @@ static int vmdk_open_se_sparse(BlockDriverState *bs,
     int ret;
     VMDKSESparseConstHeader const_header;
     VMDKSESparseVolatileHeader volatile_header;
-    VmdkExtent *extent;
+    VmdkExtent *extent = NULL;
 
     ret = bdrv_apply_auto_read_only(bs,
             "No write support for seSparse images available", errp);
@@ -869,7 +869,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
     uint32_t magic;
     uint32_t l1_size, l1_entry_sectors;
     VMDK4Header header;
-    VmdkExtent *extent;
+    VmdkExtent *extent = NULL;
     BDRVVmdkState *s = bs->opaque;
     int64_t l1_backup_offset = 0;
     bool compressed;
@@ -1088,7 +1088,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
     BdrvChild *extent_file;
     BdrvChildRole extent_role;
     BDRVVmdkState *s = bs->opaque;
-    VmdkExtent *extent;
+    VmdkExtent *extent = NULL;
     char extent_opt_prefix[32];
     Error *local_err = NULL;
 
-- 
2.26.2

[PATCH 2/4] nbd: silence maybe-uninitialized warnings

[Christian Borntraeger]

gcc 10 from Fedora 32 gives me:

Compiling C object libblock.fa.p/nbd_server.c.o
../nbd/server.c: In function ‘nbd_co_client_start’:
../nbd/server.c:625:14: error: ‘namelen’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  625 |         rc = nbd_negotiate_send_info(client, NBD_INFO_NAME, namelen, name,
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  626 |                                      errp);
      |                                      ~~~~~
../nbd/server.c:564:14: note: ‘namelen’ was declared here
  564 |     uint32_t namelen;
      |              ^~~~~~~
cc1: all warnings being treated as errors

As I cannot see how this can happen, let uns silence the warning.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 nbd/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nbd/server.c b/nbd/server.c
index 982de67816a7..2ff04ee7533d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -561,7 +561,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp)
     NBDExport *exp;
     uint16_t requests;
     uint16_t request;
-    uint32_t namelen;
+    uint32_t namelen = 0;
     bool sendname = false;
     bool blocksize = false;
     uint32_t sizes[3];
-- 
2.26.2

[PATCH 3/4] qemu-io-cmds: avoid gcc 10 warning

[Christian Borntraeger]

With gcc 10 on Fedora32 I do get:

Compiling C object libblock.fa.p/qemu-io-cmds.c.o
In file included from /usr/include/stdio.h:867,
                 from /home/cborntra/REPOS/qemu/include/qemu/osdep.h:85,
                 from ../qemu-io-cmds.c:11:
In function ‘printf’,
    inlined from ‘help_oneline’ at ../qemu-io-cmds.c:2389:9,
    inlined from ‘help_all’ at ../qemu-io-cmds.c:2414:9,
    inlined from ‘help_f’ at ../qemu-io-cmds.c:2424:9:
/usr/include/bits/stdio2.h:107:10: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
  107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Let us check for null.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 qemu-io-cmds.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index baeae86d8c85..c2080aa398a9 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2386,7 +2386,9 @@ static void help_oneline(const char *cmd, const cmdinfo_t *ct)
     if (cmd) {
         printf("%s ", cmd);
     } else {
-        printf("%s ", ct->name);
+        if (ct->name) {
+            printf("%s ", ct->name);
+        }
         if (ct->altname) {
             printf("(or %s) ", ct->altname);
         }
-- 
2.26.2

[PATCH 4/4] virtiofsd: avoid false positive compiler warning

[Christian Borntraeger]

make: *** [Makefile:121: config-host.mak] Error 1
[cborntra@m83lp52 qemu]$ make -C build/
make: Entering directory '/home/cborntra/REPOS/qemu/build'
Generating qemu-version.h with a meson_exe.py custom command
Compiling C object tools/virtiofsd/virtiofsd.p/passthrough_ll.c.o
../tools/virtiofsd/passthrough_ll.c: In function ‘lo_setattr’:
../tools/virtiofsd/passthrough_ll.c:702:19: error: ‘fd’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  702 |             res = futimens(fd, tv);
      |                   ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile.ninja:1438: tools/virtiofsd/virtiofsd.p/passthrough_ll.c.o] Error 1
make: Leaving directory '/home/cborntra/REPOS/

as far as I can see this can not happen. Let us silence the warning by
giving fd a default value.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 tools/virtiofsd/passthrough_ll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 0b229ebd5786..da06aa6e9264 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -620,7 +620,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
     struct lo_inode *inode;
     int ifd;
     int res;
-    int fd;
+    int fd = 0;
 
     inode = lo_inode(req, ino);
     if (!inode) {
-- 
2.26.2

Re: [PATCH 3/4] qemu-io-cmds: avoid gcc 10 warning

[Dr. David Alan Gilbert]

* Christian Borntraeger (borntraeger@de.ibm.com) wrote:
> With gcc 10 on Fedora32 I do get:
> 
> Compiling C object libblock.fa.p/qemu-io-cmds.c.o
> In file included from /usr/include/stdio.h:867,
>                  from /home/cborntra/REPOS/qemu/include/qemu/osdep.h:85,
>                  from ../qemu-io-cmds.c:11:
> In function ‘printf’,
>     inlined from ‘help_oneline’ at ../qemu-io-cmds.c:2389:9,
>     inlined from ‘help_all’ at ../qemu-io-cmds.c:2414:9,
>     inlined from ‘help_f’ at ../qemu-io-cmds.c:2424:9:
> /usr/include/bits/stdio2.h:107:10: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>   107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Let us check for null.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

I'd already posted
'qemu-io-cmds: Simplify help_oneline' that simplifies
this function much more; Kevin picked that up for the block branch a
couple of days ago.

Dave


> ---
>  qemu-io-cmds.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index baeae86d8c85..c2080aa398a9 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -2386,7 +2386,9 @@ static void help_oneline(const char *cmd, const cmdinfo_t *ct)
>      if (cmd) {
>          printf("%s ", cmd);
>      } else {
> -        printf("%s ", ct->name);
> +        if (ct->name) {
> +            printf("%s ", ct->name);
> +        }
>          if (ct->altname) {
>              printf("(or %s) ", ct->altname);
>          }
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 3/4] qemu-io-cmds: avoid gcc 10 warning

[Philippe Mathieu-Daudé]

On 9/30/20 5:58 PM, Christian Borntraeger wrote:
> With gcc 10 on Fedora32 I do get:
> 
> Compiling C object libblock.fa.p/qemu-io-cmds.c.o
> In file included from /usr/include/stdio.h:867,
>                  from /home/cborntra/REPOS/qemu/include/qemu/osdep.h:85,
>                  from ../qemu-io-cmds.c:11:
> In function ‘printf’,
>     inlined from ‘help_oneline’ at ../qemu-io-cmds.c:2389:9,
>     inlined from ‘help_all’ at ../qemu-io-cmds.c:2414:9,
>     inlined from ‘help_f’ at ../qemu-io-cmds.c:2424:9:
> /usr/include/bits/stdio2.h:107:10: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>   107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Let us check for null.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  qemu-io-cmds.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index baeae86d8c85..c2080aa398a9 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -2386,7 +2386,9 @@ static void help_oneline(const char *cmd, const cmdinfo_t *ct)
>      if (cmd) {
>          printf("%s ", cmd);
>      } else {
> -        printf("%s ", ct->name);
> +        if (ct->name) {
> +            printf("%s ", ct->name);
> +        }
>          if (ct->altname) {
>              printf("(or %s) ", ct->altname);
>          }
> 

This one has been fixed last month:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg732728.html

Then queued recently:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg745394.html

Re: [PATCH 4/4] virtiofsd: avoid false positive compiler warning

[Dr. David Alan Gilbert]

* Christian Borntraeger (borntraeger@de.ibm.com) wrote:
> make: *** [Makefile:121: config-host.mak] Error 1
> [cborntra@m83lp52 qemu]$ make -C build/
> make: Entering directory '/home/cborntra/REPOS/qemu/build'
> Generating qemu-version.h with a meson_exe.py custom command
> Compiling C object tools/virtiofsd/virtiofsd.p/passthrough_ll.c.o
> ../tools/virtiofsd/passthrough_ll.c: In function ‘lo_setattr’:
> ../tools/virtiofsd/passthrough_ll.c:702:19: error: ‘fd’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   702 |             res = futimens(fd, tv);
>       |                   ^~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile.ninja:1438: tools/virtiofsd/virtiofsd.p/passthrough_ll.c.o] Error 1
> make: Leaving directory '/home/cborntra/REPOS/
> 
> as far as I can see this can not happen. Let us silence the warning by
> giving fd a default value.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

Yeh, I'd posted https://www.mail-archive.com/qemu-devel@nongnu.org/msg738783.html
but not yet merged it; only difference is I'd used -1 since it seemd
safer to use -1 even if it couldn't happen :-)

Dave

> ---
>  tools/virtiofsd/passthrough_ll.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index 0b229ebd5786..da06aa6e9264 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -620,7 +620,7 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
>      struct lo_inode *inode;
>      int ifd;
>      int res;
> -    int fd;
> +    int fd = 0;
>  
>      inode = lo_inode(req, ino);
>      if (!inode) {
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 1/4] vmdk: fix maybe uninitialized warnings

[Fam Zheng]

On Wed, 2020-09-30 at 17:58 +0200, Christian Borntraeger wrote:
> Fedora 32 gcc 10 seems to give false positives:
> 
> Compiling C object libblock.fa.p/block_vmdk.c.o
> ../block/vmdk.c: In function ‘vmdk_parse_extents’:
> ../block/vmdk.c:587:5: error: ‘extent’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>   587 |     g_free(extent->l1_table);
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~
> ../block/vmdk.c:754:17: note: ‘extent’ was declared here
>   754 |     VmdkExtent *extent;
>       |                 ^~~~~~
> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>   620 |     ret = vmdk_init_tables(bs, extent, errp);
>       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>   598 |     VmdkExtent *extent;
>       |                 ^~~~~~
> ../block/vmdk.c:1178:39: error: ‘extent’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>  1178 |             extent->flat_start_offset = flat_offset << 9;
>       |             ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
> ../block/vmdk.c: In function ‘vmdk_open_vmdk4’:
> ../block/vmdk.c:581:22: error: ‘extent’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>   581 |     extent->l2_cache =
>       |     ~~~~~~~~~~~~~~~~~^
>   582 |         g_malloc(extent->entry_size * extent->l2_size *
> L2_CACHE_SIZE);
>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~
> ../block/vmdk.c:872:17: note: ‘extent’ was declared here
>   872 |     VmdkExtent *extent;
>       |                 ^~~~~~
> ../block/vmdk.c: In function ‘vmdk_open’:
> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>   620 |     ret = vmdk_init_tables(bs, extent, errp);
>       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>   598 |     VmdkExtent *extent;
>       |                 ^~~~~~
> cc1: all warnings being treated as errors
> make: *** [Makefile.ninja:884: libblock.fa.p/block_vmdk.c.o] Error 1
> 
> fix them by assigning a default value.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  block/vmdk.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 8ec62c7ab798..a00dc00eb47a 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -595,7 +595,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState
> *bs,
>      int ret;
>      uint32_t magic;
>      VMDK3Header header;
> -    VmdkExtent *extent;
> +    VmdkExtent *extent = NULL;
>  
>      ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
>      if (ret < 0) {
> @@ -751,7 +751,7 @@ static int vmdk_open_se_sparse(BlockDriverState
> *bs,
>      int ret;
>      VMDKSESparseConstHeader const_header;
>      VMDKSESparseVolatileHeader volatile_header;
> -    VmdkExtent *extent;
> +    VmdkExtent *extent = NULL;
>  
>      ret = bdrv_apply_auto_read_only(bs,
>              "No write support for seSparse images available", errp);
> @@ -869,7 +869,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>      uint32_t magic;
>      uint32_t l1_size, l1_entry_sectors;
>      VMDK4Header header;
> -    VmdkExtent *extent;
> +    VmdkExtent *extent = NULL;
>      BDRVVmdkState *s = bs->opaque;
>      int64_t l1_backup_offset = 0;
>      bool compressed;
> @@ -1088,7 +1088,7 @@ static int vmdk_parse_extents(const char *desc,
> BlockDriverState *bs,
>      BdrvChild *extent_file;
>      BdrvChildRole extent_role;
>      BDRVVmdkState *s = bs->opaque;
> -    VmdkExtent *extent;
> +    VmdkExtent *extent = NULL;
>      char extent_opt_prefix[32];
>      Error *local_err = NULL;
>  

Looks trivial, and correct.

Reviewed-by: Fam Zheng <fam@euphon.net>

Re: [PATCH 2/4] nbd: silence maybe-uninitialized warnings

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1463 bytes --]

On 9/30/20 10:58 AM, Christian Borntraeger wrote:
> gcc 10 from Fedora 32 gives me:
> 
> Compiling C object libblock.fa.p/nbd_server.c.o
> ../nbd/server.c: In function ‘nbd_co_client_start’:
> ../nbd/server.c:625:14: error: ‘namelen’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   625 |         rc = nbd_negotiate_send_info(client, NBD_INFO_NAME, namelen, name,
>       |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   626 |                                      errp);
>       |                                      ~~~~~
> ../nbd/server.c:564:14: note: ‘namelen’ was declared here
>   564 |     uint32_t namelen;
>       |              ^~~~~~~
> cc1: all warnings being treated as errors
> 
> As I cannot see how this can happen, let uns silence the warning.

gcc is smart enough to see that nbd_opt_read_name(... &namelen), which
is the only use of namelen between declaration and use, does not always
initialize namelen; but fails to see we also exit this function early in
the same conditions when nbd_opt_read_name left namelen uninit.  The
workaround is fine.

Reviewed-by: Eric Blake <eblake@redhat.com>

I'm happy for this to go in through the trivial tree, but I'll also
queue it on my NBD tree if that is ready first.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [PATCH 0/4] assorted gcc 10/fedora32 compile warning fixes

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200930155859.303148-1-borntraeger@de.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200930155859.303148-1-borntraeger@de.ibm.com
Subject: [PATCH 0/4] assorted gcc 10/fedora32 compile warning fixes

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200930043150.1454766-1-jsnow@redhat.com -> patchew/20200930043150.1454766-1-jsnow@redhat.com
 - [tag update]      patchew/20200930095321.2006-1-zhaolichang@huawei.com -> patchew/20200930095321.2006-1-zhaolichang@huawei.com
 - [tag update]      patchew/20200930151616.3588165-1-mkysel@tachyum.com -> patchew/20200930151616.3588165-1-mkysel@tachyum.com
 - [tag update]      patchew/20200930155859.303148-1-borntraeger@de.ibm.com -> patchew/20200930155859.303148-1-borntraeger@de.ibm.com
 * [new tag]         patchew/20200930164949.1425294-1-philmd@redhat.com -> patchew/20200930164949.1425294-1-philmd@redhat.com
fatal: failed to write ref-pack file
fatal: The remote end hung up unexpectedly
Traceback (most recent call last):
  File "patchew-tester/src/patchew-cli", line 521, in test_one
    git_clone_repo(clone, r["repo"], r["head"], logf, True)
  File "patchew-tester/src/patchew-cli", line 53, in git_clone_repo
    subprocess.check_call(clone_cmd, stderr=logf, stdout=logf)
  File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'clone', '-q', '/home/patchew/.cache/patchew-git-cache/httpsgithubcompatchewprojectqemu-3c8cf5a9c21ff8782164d1def7f44bd888713384', '/var/tmp/patchew-tester-tmp-529h9bha/src']' returned non-zero exit status 128.



The full log is available at
http://patchew.org/logs/20200930155859.303148-1-borntraeger@de.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 4/4] virtiofsd: avoid false positive compiler warning

[Christian Borntraeger]

On 30.09.20 18:27, Dr. David Alan Gilbert wrote:
> * Christian Borntraeger (borntraeger@de.ibm.com) wrote:
>> make: *** [Makefile:121: config-host.mak] Error 1
>> [cborntra@m83lp52 qemu]$ make -C build/
>> make: Entering directory '/home/cborntra/REPOS/qemu/build'
>> Generating qemu-version.h with a meson_exe.py custom command
>> Compiling C object tools/virtiofsd/virtiofsd.p/passthrough_ll.c.o
>> ../tools/virtiofsd/passthrough_ll.c: In function ‘lo_setattr’:
>> ../tools/virtiofsd/passthrough_ll.c:702:19: error: ‘fd’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   702 |             res = futimens(fd, tv);
>>       |                   ^~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make: *** [Makefile.ninja:1438: tools/virtiofsd/virtiofsd.p/passthrough_ll.c.o] Error 1
>> make: Leaving directory '/home/cborntra/REPOS/
>>
>> as far as I can see this can not happen. Let us silence the warning by
>> giving fd a default value.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> Yeh, I'd posted https://www.mail-archive.com/qemu-devel@nongnu.org/msg738783.html
> but not yet merged it; only difference is I'd used -1 since it seemd
> safer to use -1 even if it couldn't happen :-)

Agreed, lets go with your patch.

Re: [PATCH 2/4] nbd: silence maybe-uninitialized warnings

[Christian Borntraeger]

On 30.09.20 19:19, Eric Blake wrote:
> On 9/30/20 10:58 AM, Christian Borntraeger wrote:
>> gcc 10 from Fedora 32 gives me:
>>
>> Compiling C object libblock.fa.p/nbd_server.c.o
>> ../nbd/server.c: In function ‘nbd_co_client_start’:
>> ../nbd/server.c:625:14: error: ‘namelen’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   625 |         rc = nbd_negotiate_send_info(client, NBD_INFO_NAME, namelen, name,
>>       |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   626 |                                      errp);
>>       |                                      ~~~~~
>> ../nbd/server.c:564:14: note: ‘namelen’ was declared here
>>   564 |     uint32_t namelen;
>>       |              ^~~~~~~
>> cc1: all warnings being treated as errors
>>
>> As I cannot see how this can happen, let uns silence the warning.
> 
> gcc is smart enough to see that nbd_opt_read_name(... &namelen), which
> is the only use of namelen between declaration and use, does not always
> initialize namelen; but fails to see we also exit this function early in
> the same conditions when nbd_opt_read_name left namelen uninit.  The
> workaround is fine.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> I'm happy for this to go in through the trivial tree, but I'll also
> queue it on my NBD tree if that is ready first.

Just in case cc qemu-trival. 

Re: [PATCH 1/4] vmdk: fix maybe uninitialized warnings

[Christian Borntraeger]

On 30.09.20 18:36, Fam Zheng wrote:
> On Wed, 2020-09-30 at 17:58 +0200, Christian Borntraeger wrote:
>> Fedora 32 gcc 10 seems to give false positives:
>>
>> Compiling C object libblock.fa.p/block_vmdk.c.o
>> ../block/vmdk.c: In function ‘vmdk_parse_extents’:
>> ../block/vmdk.c:587:5: error: ‘extent’ may be used uninitialized in
>> this function [-Werror=maybe-uninitialized]
>>   587 |     g_free(extent->l1_table);
>>       |     ^~~~~~~~~~~~~~~~~~~~~~~~
>> ../block/vmdk.c:754:17: note: ‘extent’ was declared here
>>   754 |     VmdkExtent *extent;
>>       |                 ^~~~~~
>> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
>> this function [-Werror=maybe-uninitialized]
>>   620 |     ret = vmdk_init_tables(bs, extent, errp);
>>       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>>   598 |     VmdkExtent *extent;
>>       |                 ^~~~~~
>> ../block/vmdk.c:1178:39: error: ‘extent’ may be used uninitialized in
>> this function [-Werror=maybe-uninitialized]
>>  1178 |             extent->flat_start_offset = flat_offset << 9;
>>       |             ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
>> ../block/vmdk.c: In function ‘vmdk_open_vmdk4’:
>> ../block/vmdk.c:581:22: error: ‘extent’ may be used uninitialized in
>> this function [-Werror=maybe-uninitialized]
>>   581 |     extent->l2_cache =
>>       |     ~~~~~~~~~~~~~~~~~^
>>   582 |         g_malloc(extent->entry_size * extent->l2_size *
>> L2_CACHE_SIZE);
>>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ~~~~~~~~~
>> ../block/vmdk.c:872:17: note: ‘extent’ was declared here
>>   872 |     VmdkExtent *extent;
>>       |                 ^~~~~~
>> ../block/vmdk.c: In function ‘vmdk_open’:
>> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
>> this function [-Werror=maybe-uninitialized]
>>   620 |     ret = vmdk_init_tables(bs, extent, errp);
>>       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>>   598 |     VmdkExtent *extent;
>>       |                 ^~~~~~
>> cc1: all warnings being treated as errors
>> make: *** [Makefile.ninja:884: libblock.fa.p/block_vmdk.c.o] Error 1
>>
>> fix them by assigning a default value.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  block/vmdk.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 8ec62c7ab798..a00dc00eb47a 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -595,7 +595,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState
>> *bs,
>>      int ret;
>>      uint32_t magic;
>>      VMDK3Header header;
>> -    VmdkExtent *extent;
>> +    VmdkExtent *extent = NULL;
>>  
>>      ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
>>      if (ret < 0) {
>> @@ -751,7 +751,7 @@ static int vmdk_open_se_sparse(BlockDriverState
>> *bs,
>>      int ret;
>>      VMDKSESparseConstHeader const_header;
>>      VMDKSESparseVolatileHeader volatile_header;
>> -    VmdkExtent *extent;
>> +    VmdkExtent *extent = NULL;
>>  
>>      ret = bdrv_apply_auto_read_only(bs,
>>              "No write support for seSparse images available", errp);
>> @@ -869,7 +869,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>>      uint32_t magic;
>>      uint32_t l1_size, l1_entry_sectors;
>>      VMDK4Header header;
>> -    VmdkExtent *extent;
>> +    VmdkExtent *extent = NULL;
>>      BDRVVmdkState *s = bs->opaque;
>>      int64_t l1_backup_offset = 0;
>>      bool compressed;
>> @@ -1088,7 +1088,7 @@ static int vmdk_parse_extents(const char *desc,
>> BlockDriverState *bs,
>>      BdrvChild *extent_file;
>>      BdrvChildRole extent_role;
>>      BDRVVmdkState *s = bs->opaque;
>> -    VmdkExtent *extent;
>> +    VmdkExtent *extent = NULL;
>>      char extent_opt_prefix[32];
>>      Error *local_err = NULL;
>>  
> 
> Looks trivial, and correct.
> 
> Reviewed-by: Fam Zheng <fam@euphon.net>


Will this go via the block or trivial tree (cced). 

Re: [PATCH 1/4] vmdk: fix maybe uninitialized warnings

[Laurent Vivier]

Le 05/10/2020 à 08:26, Christian Borntraeger a écrit :
> On 30.09.20 18:36, Fam Zheng wrote:
>> On Wed, 2020-09-30 at 17:58 +0200, Christian Borntraeger wrote:
>>> Fedora 32 gcc 10 seems to give false positives:
>>>
>>> Compiling C object libblock.fa.p/block_vmdk.c.o
>>> ../block/vmdk.c: In function ‘vmdk_parse_extents’:
>>> ../block/vmdk.c:587:5: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>>   587 |     g_free(extent->l1_table);
>>>       |     ^~~~~~~~~~~~~~~~~~~~~~~~
>>> ../block/vmdk.c:754:17: note: ‘extent’ was declared here
>>>   754 |     VmdkExtent *extent;
>>>       |                 ^~~~~~
>>> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>>   620 |     ret = vmdk_init_tables(bs, extent, errp);
>>>       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>>>   598 |     VmdkExtent *extent;
>>>       |                 ^~~~~~
>>> ../block/vmdk.c:1178:39: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>>  1178 |             extent->flat_start_offset = flat_offset << 9;
>>>       |             ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
>>> ../block/vmdk.c: In function ‘vmdk_open_vmdk4’:
>>> ../block/vmdk.c:581:22: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>>   581 |     extent->l2_cache =
>>>       |     ~~~~~~~~~~~~~~~~~^
>>>   582 |         g_malloc(extent->entry_size * extent->l2_size *
>>> L2_CACHE_SIZE);
>>>       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> ~~~~~~~~~
>>> ../block/vmdk.c:872:17: note: ‘extent’ was declared here
>>>   872 |     VmdkExtent *extent;
>>>       |                 ^~~~~~
>>> ../block/vmdk.c: In function ‘vmdk_open’:
>>> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>>   620 |     ret = vmdk_init_tables(bs, extent, errp);
>>>       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>>>   598 |     VmdkExtent *extent;
>>>       |                 ^~~~~~
>>> cc1: all warnings being treated as errors
>>> make: *** [Makefile.ninja:884: libblock.fa.p/block_vmdk.c.o] Error 1
>>>
>>> fix them by assigning a default value.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  block/vmdk.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/block/vmdk.c b/block/vmdk.c
>>> index 8ec62c7ab798..a00dc00eb47a 100644
>>> --- a/block/vmdk.c
>>> +++ b/block/vmdk.c
>>> @@ -595,7 +595,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState
>>> *bs,
>>>      int ret;
>>>      uint32_t magic;
>>>      VMDK3Header header;
>>> -    VmdkExtent *extent;
>>> +    VmdkExtent *extent = NULL;
>>>  
>>>      ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
>>>      if (ret < 0) {
>>> @@ -751,7 +751,7 @@ static int vmdk_open_se_sparse(BlockDriverState
>>> *bs,
>>>      int ret;
>>>      VMDKSESparseConstHeader const_header;
>>>      VMDKSESparseVolatileHeader volatile_header;
>>> -    VmdkExtent *extent;
>>> +    VmdkExtent *extent = NULL;
>>>  
>>>      ret = bdrv_apply_auto_read_only(bs,
>>>              "No write support for seSparse images available", errp);
>>> @@ -869,7 +869,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>>>      uint32_t magic;
>>>      uint32_t l1_size, l1_entry_sectors;
>>>      VMDK4Header header;
>>> -    VmdkExtent *extent;
>>> +    VmdkExtent *extent = NULL;
>>>      BDRVVmdkState *s = bs->opaque;
>>>      int64_t l1_backup_offset = 0;
>>>      bool compressed;
>>> @@ -1088,7 +1088,7 @@ static int vmdk_parse_extents(const char *desc,
>>> BlockDriverState *bs,
>>>      BdrvChild *extent_file;
>>>      BdrvChildRole extent_role;
>>>      BDRVVmdkState *s = bs->opaque;
>>> -    VmdkExtent *extent;
>>> +    VmdkExtent *extent = NULL;
>>>      char extent_opt_prefix[32];
>>>      Error *local_err = NULL;
>>>  
>>
>> Looks trivial, and correct.
>>
>> Reviewed-by: Fam Zheng <fam@euphon.net>
> 
> 
> Will this go via the block or trivial tree (cced). 
> 

Applied to my trivial-patches branch.

Thanks,
Laurent