| Message ID | 20210312112143.1369-1-yuzenghui@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] multi-process: Initialize variables declared with g_auto* | expand |
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); >
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
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
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 >
[+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.
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
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);
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(-)