Re: [PATCH] capabilities: use g_autofree in capabilities.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for your review.
So, it's may be better to don's use g_autofree if its content need to be
freed manualy in cleanup?
A similiar situation could be found in  nwfilter_ebiptables_driver.c,
function 'ebiptablesApplyNewRules' assigned subchains with g_autofree
and freed its content using g_free in the cleanup. Should we unify those
situation?

On 2022/10/11 15:31, Ján Tomko wrote:
> On a Monday in 2022, Jiang Jiacheng wrote:
>> Use g_autofree in capabilities.c for some pointers still using manual
>> cleanup,
>> and remove unnecessary cleanup.
>>
>> Signed-off-by: Jiang Jiacheng <jiangjiacheng@xxxxxxxxxx>
>> ---
>> src/conf/capabilities.c | 44 +++++++++++++----------------------------
>> 1 file changed, 14 insertions(+), 30 deletions(-)
>>
>> @@ -1685,7 +1675,7 @@ static int
>> virCapabilitiesHostNUMAInitFake(virCapsHostNUMA *caps)
>> {
>>     virNodeInfo nodeinfo;
>> -    virCapsHostNUMACellCPU *cpus;
>> +    g_autofree virCapsHostNUMACellCPU *cpus;
> 
> All g_auto variable declarations must be initialized, otherwise we might
> try to free some uninitialized pointer.

I'm sorry about this mistake, I will fix it in next version.

> 
> `ninja test` complains:
> 
>>>> MALLOC_PERTURB_=46 /usr/bin/make -C
>>>> /home/jtomko/build/libvirt/build-aux
>>>> sc_require_attribute_cleanup_initialization
> ―――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ 
> ――――――――――――――――――――――――――――――――――――――――――――――――――――
> stdout:
> make: Entering directory '/home/jtomko/build/libvirt/build-aux'
> /home/jtomko/work/libvirt/src/conf/capabilities.c:1678:    g_autofree
> virCapsHostNUMACellCPU *cpus;
> make: Leaving directory '/home/jtomko/build/libvirt/build-aux'
> stderr:
> variable declared with a cleanup macro must be initialized
> make: *** [/home/jtomko/work/libvirt/build-aux/syntax-check.mk:851:
> sc_require_attribute_cleanup_initialization] Error 1
> 
> 
>>     int ncpus;
>>     int n, s, c, t;
>>     int id, cid;
>> @@ -1741,7 +1731,6 @@ virCapabilitiesHostNUMAInitFake(virCapsHostNUMA
>> *caps)
>>  error:
>>     for (; cid >= 0; cid--)
>>         virBitmapFree(cpus[cid].siblings);
>> -    VIR_FREE(cpus);
> 
> Mixing automatic cleanup of `cpus` with manual clearing of its content
> looks a bit confusing to me. virCapabilitiesClearHostNUMACellCPUTopology
> does not seem to be easily usable with g_auto, since `ncpus` is stored
> separately.
> 
> Jano
> 
>>     return -1;
>> }
>>
> 





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux