Re: [libvirt] [PATCH 1/3] virsh: modify vshStringToArray to duplicate the elements too
On 20/08/13 17:20, Peter Krempa wrote:
On 08/20/13 11:16, Osier Yang wrote:
> On 20/08/13 17:05, Peter Krempa wrote:
>> At a slightly larger memory expense allow stealing of items from the
>> string array returned from vshStringToArray and turn the result into a
>> string list compatible with virStringSplit. This will allow to use the
>> common dealloc function.
>>
>> This patch also fixes a few forgotten checks of return from
>> vshStringToArray and one memory leak.
>> ---
>> tools/virsh-nodedev.c | 18 +++++-------------
>> tools/virsh-pool.c | 10 ++++------
>> tools/virsh-snapshot.c | 10 ++--------
>> tools/virsh.c | 8 +++++---
>> tools/virsh.h | 1 +
>> 5 files changed, 17 insertions(+), 30 deletions(-)
>>
...
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index f65dc79..d6e3cb3 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -196,7 +196,8 @@ vshStringToArray(const char *str,
> Why not to destroy the use of vshStringToArray, and convert to
vshStringToArray is converting double commas (",,") as escape sequence
for a comma. virStringSplit doesn't do this. We would regress in virsh
option parsing if we'd convert it to virStringSplit.
Integrating the escaping into virStringSplit with a flag will work,
but I'm not against if you keep using it, since the mainly purpose
of these patches is to fix the bug.
> virStringSplit? and the comment of vshStringToArray should be updated,
> since the method for free'ing the memory is different now, if we keep
> using it.
Okay, I'll touch up the comment. I didn't notice it contained info how
to free it.
Peter