Re: [PATCH 2/8] virsh: Add vshDomainCompleter

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

 



On 08/19/2013 08:45 AM, Michal Privoznik wrote:
> On 08.08.2013 16:38, Tomas Meszaros wrote:
>> Function vshDomainCompler returns domains names.
>>
>> Michal Privoznik recommended to add global variable __my_conn
>> so we can get the list of domains from the virConnecTListAllDomains().
>>
>> vshReconnect() is called before the first command is executed
>> in order to provide autocompletion for the very first command.
>> ---
>>  tools/virsh.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tools/virsh.h |  2 ++
>>  2 files changed, 55 insertions(+)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index f65dc79..af31b9a 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -88,6 +88,8 @@ static char *progname;
>>  
>>  static const vshCmdGrp cmdGroups[];
>>  
>> +virConnectPtr *__my_conn;
>> +
> 
> Sigh. Unfortunately, we can't tunnel a pointer into readline's
> callbacks. While I hate global variables, this one makes sense.

Naming a variable with leading __ is not advised - too much potential
for collision with system headers.  Name it vshConn or some-such name in
the vsh namespace.

>> +
>> +    names = vshMalloc(NULL, sizeof(char *) * (ndomains + 1));
>> +
>> +    if (!names)
>> +        return NULL;
> 
> if vshMalloc fails, you leak domains[].

vshMalloc cannot fail.  If it returns at all, it returns a non-NULL
pointer (since it calls exit() on OOM).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[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]