[libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()

[Daniel Henrique Barboza]

On 12/19/19 7:17 PM, Fabiano Fidêncio wrote:
> On Thu, Dec 19, 2019 at 8:00 PM Daniel Henrique Barboza
> <danielhb413 at gmail.com> wrote:
>>
>>
>>
>> On 12/19/19 7:04 AM, Fabiano Fidêncio wrote:
>>> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
>>> ---
>>>    src/rpc/virnetclient.c | 18 ++++++------------
>>>    1 file changed, 6 insertions(+), 12 deletions(-)
>>>
>> [...]
>>
>>>        if (knownHostsPath) {
>>> @@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
>>>            goto cleanup;
>>>
>>>     cleanup:
>>> -    VIR_FREE(command);
>>> -    VIR_FREE(privkey);
>>> -    VIR_FREE(knownhosts);
>>> -    VIR_FREE(homedir);
>>> -    VIR_FREE(confdir);
>>> -    VIR_FREE(nc);
>>>        return ret;
>>>
>>
>> This will leave a now unneeded 'cleanup' label:
>>
>>    cleanup:
>>       return ret;
>>
>>
>> In a quick glance at the patch series I noticed that Patch 41 also leaves
>> an unused 'cleanup' label as well after the changes.
> 
> Daniel,
> 
> Please, take a look at the no_memory label. It calls the cleanup one.

I am afraid I wasn't clear. By 'unneeded' I mean a label that is a simply 'return'
call. 'cleanup' is doing no cleanups anymore. So this this guy:

  no_memory:
     virReportOOMError();
     goto cleanup;

turns into:

  no_memory:
     virReportOOMError();
     return ret;


And in fact, since ret is initialized with NULL, and it will never be set before
a no_memory jump, you can even do:

  no_memory:
     virReportOOMError();
     return NULL;

All 'cleanup' jumps can be changed to 'return NULL' and the line that sets 'ret'
can turn into

    return virNetClientNew(sock, NULL));

And you can get rid of both the 'cleanup' label and the 'ret' variable.


All of this is just me being pedantic though :) I saw that you erased some
'exit' labels in other patches and wanted to point out that this label is
also removable. In the end I believe you can keep this patch just adding
g_autofree and removing VIR_FREE calls and, if you want, remove these labels
in a separated patch.



Thanks,



DHB



> 
> Best Regards,
>