[PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Current QEMU HEAD nvme.c does not compile:

  hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
         trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
         ^
  hw/block/nvme.c:3150:14: note: ‘result’ was declared here
     uint32_t result;
              ^

Explicitly initialize the result to fix it.

Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- update function name in the commit message

 hw/block/nvme.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5ce21b7..c122ac0 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -3228,6 +3228,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest *req)
         result = ns->features.err_rec;
         goto out;
     case NVME_VOLATILE_WRITE_CACHE:
+        result = 0;
         for (i = 1; i <= n->num_namespaces; i++) {
             ns = nvme_ns(n, i);
             if (!ns) {
-- 
2.7.4

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Klaus Jensen]

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

CC qemu-trivial.

On Feb 10 18:23, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Current QEMU HEAD nvme.c does not compile:
> 
>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>          ^
>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>      uint32_t result;
>               ^
> 
> Explicitly initialize the result to fix it.
> 
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - update function name in the commit message
> 
>  hw/block/nvme.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5ce21b7..c122ac0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -3228,6 +3228,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest *req)
>          result = ns->features.err_rec;
>          goto out;
>      case NVME_VOLATILE_WRITE_CACHE:
> +        result = 0;
>          for (i = 1; i <= n->num_namespaces; i++) {
>              ns = nvme_ns(n, i);
>              if (!ns) {
> -- 
> 2.7.4
> 
> 

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

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Minwoo Im]

On 21-02-10 18:23:17, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Current QEMU HEAD nvme.c does not compile:
> 
>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>          ^
>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>      uint32_t result;
>               ^
> 
> Explicitly initialize the result to fix it.
> 
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Bin,

Thanks for the fix!

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Philippe Mathieu-Daudé]

Hi Bin,

On 2/10/21 11:23 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Current QEMU HEAD nvme.c does not compile:
> 
>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>          ^
>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>      uint32_t result;
>               ^

Why isn't this catched by our CI? What is your host OS? Fedora 33?

> 
> Explicitly initialize the result to fix it.
> 
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - update function name in the commit message
> 
>  hw/block/nvme.c | 1 +
>  1 file changed, 1 insertion(+)

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Bin Meng]

Hi Philippe,

On Wed, Feb 10, 2021 at 7:12 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Hi Bin,
>
> On 2/10/21 11:23 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Current QEMU HEAD nvme.c does not compile:
> >
> >   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> >          ^
> >   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> >      uint32_t result;
> >               ^
>
> Why isn't this catched by our CI? What is your host OS? Fedora 33?
>

I am using GCC 5.4 on Ubuntu 16.04. Please see some initial analysis
from Peter about why newer version GCC does not report it.

Regards,
Bin

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Philippe Mathieu-Daudé]

On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> Hi Bin,
> 
> On 2/10/21 11:23 AM, Bin Meng wrote:
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> Current QEMU HEAD nvme.c does not compile:
>>
>>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>>          ^
>>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>>      uint32_t result;
>>               ^
> 
> Why isn't this catched by our CI? What is your host OS? Fedora 33?

Just noticed v1 and Peter's explanation:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html

Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
please?

> 
>>
>> Explicitly initialize the result to fix it.
>>
>> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>
>> ---
>>
>> Changes in v2:
>> - update function name in the commit message
>>
>>  hw/block/nvme.c | 1 +
>>  1 file changed, 1 insertion(+)

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Bin Meng]

Hi Philippe,

On Wed, Feb 10, 2021 at 7:15 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> > Hi Bin,
> >
> > On 2/10/21 11:23 AM, Bin Meng wrote:
> >> From: Bin Meng <bin.meng@windriver.com>
> >>
> >> Current QEMU HEAD nvme.c does not compile:
> >>
> >>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> >>          ^
> >>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> >>      uint32_t result;
> >>               ^
> >
> > Why isn't this catched by our CI? What is your host OS? Fedora 33?
>
> Just noticed v1 and Peter's explanation:
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html
>
> Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
> please?
>

Sure.

Regards,
Bin

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Daniel P. Berrangé]

On Wed, Feb 10, 2021 at 12:15:45PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> > Hi Bin,
> > 
> > On 2/10/21 11:23 AM, Bin Meng wrote:
> >> From: Bin Meng <bin.meng@windriver.com>
> >>
> >> Current QEMU HEAD nvme.c does not compile:
> >>
> >>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> >>          ^
> >>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> >>      uint32_t result;
> >>               ^
> > 
> > Why isn't this catched by our CI? What is your host OS? Fedora 33?
> 
> Just noticed v1 and Peter's explanation:
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html
> 
> Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
> please?

Well Ubuntu 16.04 hasn't been considered a supported build target for
QEMU for a year now.

https://qemu.readthedocs.io/en/latest/system/build-platforms.html#linux-os-macos-freebsd-netbsd-openbsd

  "The project aims to support the most recent major version 
   at all times. Support for the previous major version will 
   be dropped 2 years after the new major version is released
   or when the vendor itself drops support, whichever comes 
   first."

IOW, we only aim for QEMU to be buildable on Ubuntu LTS 20.04 and 18.04
at this point in time.  16.04 is explicitly dropped and we will increasingly
introduce incompatibilities with it.

While this specific patch is simple, trying to keep QEMU git master
working on 16.04 is not a goal, so I'd really suggest upgrading to
a newer Ubuntu version at the soonest opportunity.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Thomas Huth]

On 10/02/2021 12.15, Bin Meng wrote:
> Hi Philippe,
> 
> On Wed, Feb 10, 2021 at 7:12 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> Hi Bin,
>>
>> On 2/10/21 11:23 AM, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@windriver.com>
>>>
>>> Current QEMU HEAD nvme.c does not compile:
>>>
>>>    hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>           trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>>>           ^
>>>    hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>>>       uint32_t result;
>>>                ^
>>
>> Why isn't this catched by our CI? What is your host OS? Fedora 33?
>>
> 
> I am using GCC 5.4 on Ubuntu 16.04. Please see some initial analysis
> from Peter about why newer version GCC does not report it.

Please note that Ubuntu 16.04 is not a supported version by QEMU anymore, 
see https://qemu.readthedocs.io/en/latest/system/build-platforms.html and 
https://wiki.qemu.org/Supported_Build_Platforms for details.

  Thomas

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Daniel P. Berrangé]

On Wed, Feb 10, 2021 at 11:22:19AM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 10, 2021 at 12:15:45PM +0100, Philippe Mathieu-Daudé wrote:
> > On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> > > Hi Bin,
> > > 
> > > On 2/10/21 11:23 AM, Bin Meng wrote:
> > >> From: Bin Meng <bin.meng@windriver.com>
> > >>
> > >> Current QEMU HEAD nvme.c does not compile:
> > >>
> > >>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> > >>          ^
> > >>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> > >>      uint32_t result;
> > >>               ^
> > > 
> > > Why isn't this catched by our CI? What is your host OS? Fedora 33?
> > 
> > Just noticed v1 and Peter's explanation:
> > https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html
> > 
> > Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
> > please?
> 
> Well Ubuntu 16.04 hasn't been considered a supported build target for
> QEMU for a year now.
> 
> https://qemu.readthedocs.io/en/latest/system/build-platforms.html#linux-os-macos-freebsd-netbsd-openbsd
> 
>   "The project aims to support the most recent major version 
>    at all times. Support for the previous major version will 
>    be dropped 2 years after the new major version is released
>    or when the vendor itself drops support, whichever comes 
>    first."
> 
> IOW, we only aim for QEMU to be buildable on Ubuntu LTS 20.04 and 18.04
> at this point in time.  16.04 is explicitly dropped and we will increasingly
> introduce incompatibilities with it.
> 
> While this specific patch is simple, trying to keep QEMU git master
> working on 16.04 is not a goal, so I'd really suggest upgrading to
> a newer Ubuntu version at the soonest opportunity.

In particular after 6.0 QEMU is released, we'll be dropping RHEL-7
and then likely setting the  min required GCC to somewhere around
6.3 which will cut off Ubuntu 16.04 upfront.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()

[Peter Maydell]

On Wed, 10 Feb 2021 at 10:23, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Current QEMU HEAD nvme.c does not compile:
>
>   hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>          trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>          ^
>   hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>      uint32_t result;
>               ^
>
> Explicitly initialize the result to fix it.
>
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v2:
> - update function name in the commit message
>
>  hw/block/nvme.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5ce21b7..c122ac0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -3228,6 +3228,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest *req)
>          result = ns->features.err_rec;
>          goto out;
>      case NVME_VOLATILE_WRITE_CACHE:
> +        result = 0;
>          for (i = 1; i <= n->num_namespaces; i++) {
>              ns = nvme_ns(n, i);
>              if (!ns) {
> --

Also spotted by Coverity: CID 1446371

-- PMM