* [PATCH] multi-process: Initialize variables declared with g_auto*
@ 2021-03-03 7:06 Zenghui Yu
2021-03-03 8:44 ` Jag Raman
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Zenghui Yu @ 2021-03-03 7:06 UTC (permalink / raw)
To: elena.ufimtseva, jag.raman, john.g.johnson
Cc: Zenghui Yu, wanghaibin.wang, berrange, qemu-devel
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:
../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>
---
hw/remote/memory.c | 3 +--
hw/remote/proxy.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/remote/memory.c b/hw/remote/memory.c
index 32085b1e05..f5f735c15a 100644
--- a/hw/remote/memory.c
+++ b/hw/remote/memory.c
@@ -43,9 +43,8 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
remote_sysmem_reset();
for (region = 0; region < msg->num_fds; region++) {
- g_autofree char *name;
+ 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] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 7:06 [PATCH] multi-process: Initialize variables declared with g_auto* Zenghui Yu
@ 2021-03-03 8:44 ` Jag Raman
2021-03-03 10:08 ` Philippe Mathieu-Daudé
2021-03-03 10:17 ` Daniel P. Berrangé
2 siblings, 0 replies; 9+ messages in thread
From: Jag Raman @ 2021-03-03 8:44 UTC (permalink / raw)
To: Zenghui Yu
Cc: Elena Ufimtseva, John Johnson, berrange, qemu-devel, wanghaibin.wang
> On Mar 3, 2021, at 2:06 AM, Zenghui Yu <yuzenghui@huawei.com> 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:
>
> ../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>
> ---
> hw/remote/memory.c | 3 +--
> hw/remote/proxy.c | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/remote/memory.c b/hw/remote/memory.c
> index 32085b1e05..f5f735c15a 100644
> --- a/hw/remote/memory.c
> +++ b/hw/remote/memory.c
> @@ -43,9 +43,8 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
> remote_sysmem_reset();
>
> for (region = 0; region < msg->num_fds; region++) {
> - g_autofree char *name;
> + 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
Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 7:06 [PATCH] multi-process: Initialize variables declared with g_auto* Zenghui Yu
2021-03-03 8:44 ` Jag Raman
@ 2021-03-03 10:08 ` Philippe Mathieu-Daudé
2021-03-03 10:17 ` Daniel P. Berrangé
2 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-03 10:08 UTC (permalink / raw)
To: Zenghui Yu, elena.ufimtseva, jag.raman, john.g.johnson
Cc: wanghaibin.wang, berrange, qemu-devel
Hi,
On 3/3/21 8:06 AM, 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:
>
> ../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>
> ---
> hw/remote/memory.c | 3 +--
> hw/remote/proxy.c | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/remote/memory.c b/hw/remote/memory.c
> index 32085b1e05..f5f735c15a 100644
> --- a/hw/remote/memory.c
> +++ b/hw/remote/memory.c
> @@ -43,9 +43,8 @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
> remote_sysmem_reset();
>
> for (region = 0; region < msg->num_fds; region++) {
Could you move the suffix iteration out of the loop?
for (region = 0; region < msg->num_fds; region++, suffix++) {
> - g_autofree char *name;
> + 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],
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 7:06 [PATCH] multi-process: Initialize variables declared with g_auto* Zenghui Yu
2021-03-03 8:44 ` Jag Raman
2021-03-03 10:08 ` Philippe Mathieu-Daudé
@ 2021-03-03 10:17 ` Daniel P. Berrangé
2021-03-03 10:26 ` Philippe Mathieu-Daudé
` (2 more replies)
2 siblings, 3 replies; 9+ messages in thread
From: Daniel P. Berrangé @ 2021-03-03 10:17 UTC (permalink / raw)
To: Zenghui Yu
Cc: elena.ufimtseva, john.g.johnson, jag.raman, qemu-devel, wanghaibin.wang
On Wed, Mar 03, 2021 at 03:06:39PM +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:
>
> ../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;
> ^~~~
This is a bit wierd. There should only be risk of uninitialized
variable if there is a 'return' or 'goto' statement between the
variable declaration and and initialization, which is not the
case in either scenario here.
What OS distro and compiler + version are you seeing this with ?
Also we seem to be lacking any gitlab CI job to test with the
multiprocess feature enabled
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 :|
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 10:17 ` Daniel P. Berrangé
@ 2021-03-03 10:26 ` Philippe Mathieu-Daudé
2021-03-03 14:24 ` Jag Raman
2021-03-04 2:12 ` Zenghui Yu
2 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-03 10:26 UTC (permalink / raw)
To: Daniel P. Berrangé, Zenghui Yu, Denis Plotnikov
Cc: elena.ufimtseva, john.g.johnson, jag.raman, qemu-devel, wanghaibin.wang
On 3/3/21 11:17 AM, Daniel P. Berrangé wrote:
> On Wed, Mar 03, 2021 at 03:06:39PM +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:
>>
>> ../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;
>> ^~~~
>
> This is a bit wierd. There should only be risk of uninitialized
> variable if there is a 'return' or 'goto' statement between the
> variable declaration and and initialization, which is not the
> case in either scenario here.
See also commit 076b2fadb58 ("gdbstub: fix compiler complaining").
>
> What OS distro and compiler + version are you seeing this with ?
>
> Also we seem to be lacking any gitlab CI job to test with the
> multiprocess feature enabled
>
> Regards,
> Daniel
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 10:17 ` Daniel P. Berrangé
2021-03-03 10:26 ` Philippe Mathieu-Daudé
@ 2021-03-03 14:24 ` Jag Raman
2021-03-03 14:26 ` Daniel P. Berrangé
2021-03-04 2:12 ` Zenghui Yu
2 siblings, 1 reply; 9+ messages in thread
From: Jag Raman @ 2021-03-03 14:24 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Zenghui Yu, Elena Ufimtseva, qemu-devel, wanghaibin.wang, John Johnson
> On Mar 3, 2021, at 5:17 AM, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Mar 03, 2021 at 03:06:39PM +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:
>>
>> ../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;
>> ^~~~
>
> This is a bit wierd. There should only be risk of uninitialized
> variable if there is a 'return' or 'goto' statement between the
> variable declaration and and initialization, which is not the
> case in either scenario here.
>
> What OS distro and compiler + version are you seeing this with ?
>
> Also we seem to be lacking any gitlab CI job to test with the
> multiprocess feature enabled
Hi Daniel,
Concerning gitlab CI, it looks like we are running acceptance tests as
part of it. "acceptance-system-fedora" runs it on fedora.
Is it sufficient to have multiprocess tests as part acceptance tests suite
or do you prefer to have a separate test in gitlab CI?
Thank you!
—
Jag
>
> 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 :|
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 14:24 ` Jag Raman
@ 2021-03-03 14:26 ` Daniel P. Berrangé
2021-03-03 14:28 ` Jag Raman
0 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrangé @ 2021-03-03 14:26 UTC (permalink / raw)
To: Jag Raman
Cc: Zenghui Yu, Elena Ufimtseva, qemu-devel, wanghaibin.wang, John Johnson
On Wed, Mar 03, 2021 at 02:24:04PM +0000, Jag Raman wrote:
>
>
> > On Mar 3, 2021, at 5:17 AM, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Mar 03, 2021 at 03:06:39PM +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:
> >>
> >> ../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;
> >> ^~~~
> >
> > This is a bit wierd. There should only be risk of uninitialized
> > variable if there is a 'return' or 'goto' statement between the
> > variable declaration and and initialization, which is not the
> > case in either scenario here.
> >
> > What OS distro and compiler + version are you seeing this with ?
> >
> > Also we seem to be lacking any gitlab CI job to test with the
> > multiprocess feature enabled
>
> Hi Daniel,
>
> Concerning gitlab CI, it looks like we are running acceptance tests as
> part of it. "acceptance-system-fedora" runs it on fedora.
>
> Is it sufficient to have multiprocess tests as part acceptance tests suite
> or do you prefer to have a separate test in gitlab CI?
No problem. it is just me getting confused. I was looking for a CI job
with --enable-multiprocess, not realizing it is enabled by default
on Linux in configure
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 :|
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 14:26 ` Daniel P. Berrangé
@ 2021-03-03 14:28 ` Jag Raman
0 siblings, 0 replies; 9+ messages in thread
From: Jag Raman @ 2021-03-03 14:28 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Zenghui Yu, Elena Ufimtseva, qemu-devel, wanghaibin.wang, John Johnson
> On Mar 3, 2021, at 9:26 AM, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Mar 03, 2021 at 02:24:04PM +0000, Jag Raman wrote:
>>
>>
>>> On Mar 3, 2021, at 5:17 AM, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>
>>> On Wed, Mar 03, 2021 at 03:06:39PM +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:
>>>>
>>>> ../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;
>>>> ^~~~
>>>
>>> This is a bit wierd. There should only be risk of uninitialized
>>> variable if there is a 'return' or 'goto' statement between the
>>> variable declaration and and initialization, which is not the
>>> case in either scenario here.
>>>
>>> What OS distro and compiler + version are you seeing this with ?
>>>
>>> Also we seem to be lacking any gitlab CI job to test with the
>>> multiprocess feature enabled
>>
>> Hi Daniel,
>>
>> Concerning gitlab CI, it looks like we are running acceptance tests as
>> part of it. "acceptance-system-fedora" runs it on fedora.
>>
>> Is it sufficient to have multiprocess tests as part acceptance tests suite
>> or do you prefer to have a separate test in gitlab CI?
>
> No problem. it is just me getting confused. I was looking for a CI job
> with --enable-multiprocess, not realizing it is enabled by default
> on Linux in configure
Thank you for confirming!
—
Jag
>
>
> 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 :|
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] multi-process: Initialize variables declared with g_auto*
2021-03-03 10:17 ` Daniel P. Berrangé
2021-03-03 10:26 ` Philippe Mathieu-Daudé
2021-03-03 14:24 ` Jag Raman
@ 2021-03-04 2:12 ` Zenghui Yu
2 siblings, 0 replies; 9+ messages in thread
From: Zenghui Yu @ 2021-03-04 2:12 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: elena.ufimtseva, john.g.johnson, jag.raman, qemu-devel, wanghaibin.wang
On 2021/3/3 18:17, Daniel P. Berrangé wrote:
> This is a bit wierd. There should only be risk of uninitialized
> variable if there is a 'return' or 'goto' statement between the
> variable declaration and and initialization, which is not the
> case in either scenario here.
>
> What OS distro and compiler + version are you seeing this with ?
This was noticed when compiling QEMU with gcc-7.3.0 on CentOS.
Thanks,
Zenghui
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-04 2:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03 7:06 [PATCH] multi-process: Initialize variables declared with g_auto* Zenghui Yu
2021-03-03 8:44 ` Jag Raman
2021-03-03 10:08 ` Philippe Mathieu-Daudé
2021-03-03 10:17 ` Daniel P. Berrangé
2021-03-03 10:26 ` Philippe Mathieu-Daudé
2021-03-03 14:24 ` Jag Raman
2021-03-03 14:26 ` Daniel P. Berrangé
2021-03-03 14:28 ` Jag Raman
2021-03-04 2:12 ` Zenghui Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.