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

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



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


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