[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] multi-process: Initialize variables declared with g_auto*
From: |
Jag Raman |
Subject: |
Re: [PATCH] multi-process: Initialize variables declared with g_auto* |
Date: |
Wed, 3 Mar 2021 14:28:41 +0000 |
> 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 :|
>