Re: [PATCH 4/4] src: Use g_steal_pointer() more

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

 



On 2/1/22 17:18, Erik Skultety wrote:
> On Mon, Jan 31, 2022 at 03:53:42PM +0100, Michal Privoznik wrote:
>> There are few places where the g_steal_pointer() is open coded.
>> Switch them to calling the g_steal_pointer() function instead.
>> Generated by the following spatch:
>>
>>   @ rule1 @
>>   expression a, b;
>>   @@
>>     <...
>>   - b = a;
>>     ... when != b
>>   - a = NULL;
>>   + b = g_steal_pointer(&a);
>>     ...>
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
> ...
> 
>> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
>> index 7b684e04ba..52851ac507 100644
>> --- a/src/hyperv/hyperv_driver.c
>> +++ b/src/hyperv/hyperv_driver.c
>> @@ -1780,8 +1780,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>>  
>>      priv->version = g_strdup(os->data->Version);
>>  
>> -    conn->privateData = priv;
>> -    priv = NULL;
>> +    conn->privateData = g_steal_pointer(&priv);
>>      result = VIR_DRV_OPEN_SUCCESS;
>>  
>>   cleanup:
>> @@ -3518,9 +3517,8 @@ hypervConnectListAllDomains(virConnectPtr conn,
>>          doms[count++] = domain;
>>      }
>>  
>> -    if (doms)
>> -        *domains = doms;
>> -    doms = NULL;
>> +    if (domains)
>> +        *domains = g_steal_pointer(&doms);
> 
> ^this is not semantically identical, you need to fix it manually before pushing
> 
> Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
> 

Yeah, hyperv code doesn't get as much love as other drivers, but
basically, @domains is allocated iff @doms != NULL, there's the
following code earlier in the function:

    if (domains) {
        doms = g_new0(virDomainPtr, 1);
        ndoms = 1;
    }

I find the new version easier to read, since @domains is an argument of
the function. Do you want me to post a separate patch that does
s/doms/domains/?

Michal




[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