* [PATCH v3] multi-process: Initialize variables declared with g_auto*
@ 2021-03-12 11:21 Zenghui Yu
2021-03-12 13:54 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Zenghui Yu @ 2021-03-12 11:21 UTC (permalink / raw)
To: elena.ufimtseva, jag.raman, john.g.johnson
Cc: berrange, qemu-trivial, mjt, qemu-devel, laurent, Zenghui Yu,
wanghaibin.wang, philmd
Quote docs/devel/style.rst (section "Automatic memory deallocation"):
* Variables declared with g_auto* MUST always be initialized,
otherwise the cleanup function will use uninitialized stack memory
Initialize @name properly to get rid of the compilation error (using
gcc-7.3.0 on CentOS):
../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
g_free (*pp);
^~~~~~~~~~~~
../hw/remote/proxy.c:350:30: note: 'name' was declared here
g_autofree char *name;
^~~~
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
* From v2:
- Add OS distro and compiler version into commit message
- Add Philippe's R-b
- Cc: qemu-trivial@nongnu.org
hw/remote/memory.c | 5 ++---
hw/remote/proxy.c | 3 +--
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/remote/memory.c b/hw/remote/memory.c
index 32085b1e05..d97947d4b8 100644
--- a/hw/remote/memory.c
+++ b/hw/remote/memory.c
@@ -42,10 +42,9 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
remote_sysmem_reset();
- for (region = 0; region < msg->num_fds; region++) {
- g_autofree char *name;
+ for (region = 0; region < msg->num_fds; region++, suffix++) {
+ g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
subregion = g_new(MemoryRegion, 1);
- name = g_strdup_printf("remote-mem-%u", suffix++);
memory_region_init_ram_from_fd(subregion, NULL,
name, sysmem_info->sizes[region],
true, msg->fds[region],
diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
index 4fa4be079d..6dda705fc2 100644
--- a/hw/remote/proxy.c
+++ b/hw/remote/proxy.c
@@ -347,13 +347,12 @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
if (size) {
- g_autofree char *name;
+ g_autofree char *name = g_strdup_printf("bar-region-%d", i);
pdev->region[i].dev = pdev;
pdev->region[i].present = true;
if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
pdev->region[i].memory = true;
}
- name = g_strdup_printf("bar-region-%d", i);
memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
&proxy_mr_ops, &pdev->region[i],
name, size);
--
2.19.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] multi-process: Initialize variables declared with g_auto*
2021-03-12 11:21 [PATCH v3] multi-process: Initialize variables declared with g_auto* Zenghui Yu
@ 2021-03-12 13:54 ` Philippe Mathieu-Daudé
2021-03-15 5:48 ` Miroslav Rezanina
2021-04-06 14:00 ` Zenghui Yu
2 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-12 13:54 UTC (permalink / raw)
To: Zenghui Yu, elena.ufimtseva, jag.raman, john.g.johnson
Cc: berrange, qemu-trivial, mjt, qemu-devel, laurent,
wanghaibin.wang, Miroslav Rezanina
Cc'ing Miroslav
On 3/12/21 12:21 PM, Zenghui Yu wrote:
> Quote docs/devel/style.rst (section "Automatic memory deallocation"):
>
> * Variables declared with g_auto* MUST always be initialized,
> otherwise the cleanup function will use uninitialized stack memory
>
> Initialize @name properly to get rid of the compilation error (using
> gcc-7.3.0 on CentOS):
>
> ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
> /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> g_free (*pp);
> ^~~~~~~~~~~~
> ../hw/remote/proxy.c:350:30: note: 'name' was declared here
> g_autofree char *name;
> ^~~~
>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> * From v2:
> - Add OS distro and compiler version into commit message
> - Add Philippe's R-b
> - Cc: qemu-trivial@nongnu.org
>
> hw/remote/memory.c | 5 ++---
> hw/remote/proxy.c | 3 +--
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/remote/memory.c b/hw/remote/memory.c
> index 32085b1e05..d97947d4b8 100644
> --- a/hw/remote/memory.c
> +++ b/hw/remote/memory.c
> @@ -42,10 +42,9 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
>
> remote_sysmem_reset();
>
> - for (region = 0; region < msg->num_fds; region++) {
> - g_autofree char *name;
> + for (region = 0; region < msg->num_fds; region++, suffix++) {
> + g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
> subregion = g_new(MemoryRegion, 1);
> - name = g_strdup_printf("remote-mem-%u", suffix++);
> memory_region_init_ram_from_fd(subregion, NULL,
> name, sysmem_info->sizes[region],
> true, msg->fds[region],
> diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
> index 4fa4be079d..6dda705fc2 100644
> --- a/hw/remote/proxy.c
> +++ b/hw/remote/proxy.c
> @@ -347,13 +347,12 @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
> PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
>
> if (size) {
> - g_autofree char *name;
> + g_autofree char *name = g_strdup_printf("bar-region-%d", i);
> pdev->region[i].dev = pdev;
> pdev->region[i].present = true;
> if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
> pdev->region[i].memory = true;
> }
> - name = g_strdup_printf("bar-region-%d", i);
> memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
> &proxy_mr_ops, &pdev->region[i],
> name, size);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] multi-process: Initialize variables declared with g_auto*
2021-03-12 11:21 [PATCH v3] multi-process: Initialize variables declared with g_auto* Zenghui Yu
2021-03-12 13:54 ` Philippe Mathieu-Daudé
@ 2021-03-15 5:48 ` Miroslav Rezanina
2021-03-15 6:20 ` Zenghui Yu
2021-04-06 14:00 ` Zenghui Yu
2 siblings, 1 reply; 7+ messages in thread
From: Miroslav Rezanina @ 2021-03-15 5:48 UTC (permalink / raw)
To: Zenghui Yu
Cc: elena.ufimtseva, john.g.johnson, jag.raman, qemu-trivial, mjt,
qemu-devel, laurent, wanghaibin.wang, berrange, philmd
On Fri, Mar 12, 2021 at 07:21:43PM +0800, Zenghui Yu wrote:
> Quote docs/devel/style.rst (section "Automatic memory deallocation"):
>
> * Variables declared with g_auto* MUST always be initialized,
> otherwise the cleanup function will use uninitialized stack memory
>
> Initialize @name properly to get rid of the compilation error (using
> gcc-7.3.0 on CentOS):
>
> ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
> /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> g_free (*pp);
> ^~~~~~~~~~~~
> ../hw/remote/proxy.c:350:30: note: 'name' was declared here
> g_autofree char *name;
> ^~~~
>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> * From v2:
> - Add OS distro and compiler version into commit message
> - Add Philippe's R-b
> - Cc: qemu-trivial@nongnu.org
>
> hw/remote/memory.c | 5 ++---
> hw/remote/proxy.c | 3 +--
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/remote/memory.c b/hw/remote/memory.c
> index 32085b1e05..d97947d4b8 100644
> --- a/hw/remote/memory.c
> +++ b/hw/remote/memory.c
> @@ -42,10 +42,9 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
>
> remote_sysmem_reset();
>
> - for (region = 0; region < msg->num_fds; region++) {
> - g_autofree char *name;
> + for (region = 0; region < msg->num_fds; region++, suffix++) {
> + g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);
> subregion = g_new(MemoryRegion, 1);
> - name = g_strdup_printf("remote-mem-%u", suffix++);
> memory_region_init_ram_from_fd(subregion, NULL,
> name, sysmem_info->sizes[region],
> true, msg->fds[region],
> diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c
> index 4fa4be079d..6dda705fc2 100644
> --- a/hw/remote/proxy.c
> +++ b/hw/remote/proxy.c
> @@ -347,13 +347,12 @@ static void probe_pci_info(PCIDevice *dev, Error **errp)
> PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;
>
> if (size) {
> - g_autofree char *name;
> + g_autofree char *name = g_strdup_printf("bar-region-%d", i);
> pdev->region[i].dev = pdev;
> pdev->region[i].present = true;
> if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {
> pdev->region[i].memory = true;
> }
> - name = g_strdup_printf("bar-region-%d", i);
> memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),
> &proxy_mr_ops, &pdev->region[i],
> name, size);
> --
> 2.19.1
>
>
Missing declaration without initialization in hw/s390x/s390-pci-vfio.c
othwerwise correct. Will you send v4 with missing initialization or
should I send then as another patch?
Mirek
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] multi-process: Initialize variables declared with g_auto*
2021-03-15 5:48 ` Miroslav Rezanina
@ 2021-03-15 6:20 ` Zenghui Yu
2021-03-15 6:32 ` Miroslav Rezanina
0 siblings, 1 reply; 7+ messages in thread
From: Zenghui Yu @ 2021-03-15 6:20 UTC (permalink / raw)
To: Miroslav Rezanina
Cc: elena.ufimtseva, john.g.johnson, jag.raman, qemu-trivial, mjt,
qemu-devel, laurent, wanghaibin.wang, berrange, philmd
On 2021/3/15 13:48, Miroslav Rezanina wrote:
> Missing declaration without initialization in hw/s390x/s390-pci-vfio.c
> othwerwise correct. Will you send v4 with missing initialization or
> should I send then as another patch?
I'd prefer the latter so that subsystem maintainers can take the
separate patch into their own tree ('Multi-process QEMU' and 'S390 PCI'
in this case). Please go ahead for the s390 fix.
Thanks,
Zenghui
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] multi-process: Initialize variables declared with g_auto*
2021-03-15 6:20 ` Zenghui Yu
@ 2021-03-15 6:32 ` Miroslav Rezanina
0 siblings, 0 replies; 7+ messages in thread
From: Miroslav Rezanina @ 2021-03-15 6:32 UTC (permalink / raw)
To: Zenghui Yu
Cc: elena.ufimtseva, john.g.johnson, jag.raman, qemu-trivial, mjt,
qemu-devel, laurent, berrange, wanghaibin.wang, philmd
On Mon, Mar 15, 2021 at 02:20:10PM +0800, Zenghui Yu wrote:
> On 2021/3/15 13:48, Miroslav Rezanina wrote:
> > Missing declaration without initialization in hw/s390x/s390-pci-vfio.c
> > othwerwise correct. Will you send v4 with missing initialization or
> > should I send then as another patch?
>
> I'd prefer the latter so that subsystem maintainers can take the
> separate patch into their own tree ('Multi-process QEMU' and 'S390 PCI'
> in this case). Please go ahead for the s390 fix.
>
Ok,
I'll handle remaining g_autofree.
Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>
>
> Thanks,
> Zenghui
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] multi-process: Initialize variables declared with g_auto*
2021-03-12 11:21 [PATCH v3] multi-process: Initialize variables declared with g_auto* Zenghui Yu
2021-03-12 13:54 ` Philippe Mathieu-Daudé
2021-03-15 5:48 ` Miroslav Rezanina
@ 2021-04-06 14:00 ` Zenghui Yu
2021-04-26 13:30 ` Stefan Hajnoczi
2 siblings, 1 reply; 7+ messages in thread
From: Zenghui Yu @ 2021-04-06 14:00 UTC (permalink / raw)
To: elena.ufimtseva, jag.raman, john.g.johnson, stefanha
Cc: berrange, qemu-trivial, mjt, qemu-devel, laurent,
wanghaibin.wang, philmd
[+Stefan]
On 2021/3/12 19:21, Zenghui Yu wrote:
> Quote docs/devel/style.rst (section "Automatic memory deallocation"):
>
> * Variables declared with g_auto* MUST always be initialized,
> otherwise the cleanup function will use uninitialized stack memory
>
> Initialize @name properly to get rid of the compilation error (using
> gcc-7.3.0 on CentOS):
>
> ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
> /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> g_free (*pp);
> ^~~~~~~~~~~~
> ../hw/remote/proxy.c:350:30: note: 'name' was declared here
> g_autofree char *name;
> ^~~~
>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210312112143.1369-1-yuzenghui@huawei.com>
Ping for 6.0, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] multi-process: Initialize variables declared with g_auto*
2021-04-06 14:00 ` Zenghui Yu
@ 2021-04-26 13:30 ` Stefan Hajnoczi
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-04-26 13:30 UTC (permalink / raw)
To: Zenghui Yu
Cc: elena.ufimtseva, john.g.johnson, jag.raman, qemu-trivial,
qemu-stable, mjt, qemu-devel, laurent, stefanha, wanghaibin.wang,
berrange, philmd
[-- Attachment #1: Type: text/plain, Size: 2267 bytes --]
On Tue, Apr 06, 2021 at 10:00:03PM +0800, Zenghui Yu wrote:
> [+Stefan]
>
> On 2021/3/12 19:21, Zenghui Yu wrote:
> > Quote docs/devel/style.rst (section "Automatic memory deallocation"):
> >
> > * Variables declared with g_auto* MUST always be initialized,
> > otherwise the cleanup function will use uninitialized stack memory
> >
> > Initialize @name properly to get rid of the compilation error (using
> > gcc-7.3.0 on CentOS):
> >
> > ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':
> > /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > g_free (*pp);
> > ^~~~~~~~~~~~
> > ../hw/remote/proxy.c:350:30: note: 'name' was declared here
> > g_autofree char *name;
> > ^~~~
> >
> > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> > Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Message-Id: <20210312112143.1369-1-yuzenghui@huawei.com>
>
> Ping for 6.0, thanks.
I'm sorry I missed this email! QEMU 6.0.0-rc4 has already been tagged
and the final release is tomorrow. Only critical patches are applied at
this stage of the release process.
My understanding is that this patch silences a gcc 7.3.0 warning. The
warning is bogus since both code paths always initialize the variable.
You should still be able to compile successfully using ./configure
--disable-werror.
I guess this issue was hit on CentOS 7 since CentOS 8 ships a newer
version of gcc. Debian stable also ships a newer gcc. That probably
explains why this issue has not been encountered by others. I don't
think the patch is absolutely critical for QEMU 6.0 although I regret
not having merged it earlier in the release process. Sorry again.
I have queued this patch for QEMU 6.1 and CCed it for -stable for
inclusion in QEMU 6.0.1.
If you think this patch is critical for QEMU 6.0, please get in contact
with myself and Peter Maydel ("pm215"), preferrably on #qemu
irc.oftc.net IRC as soon as possible.
Thanks, applied to my block-next tree:
https://gitlab.com/stefanha/qemu/commits/block-next
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-26 13:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 11:21 [PATCH v3] multi-process: Initialize variables declared with g_auto* Zenghui Yu
2021-03-12 13:54 ` Philippe Mathieu-Daudé
2021-03-15 5:48 ` Miroslav Rezanina
2021-03-15 6:20 ` Zenghui Yu
2021-03-15 6:32 ` Miroslav Rezanina
2021-04-06 14:00 ` Zenghui Yu
2021-04-26 13:30 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).