[libvirt] [libvirt-php][PATCH 2/3] Fix some leaks related to get_string_from_xpath

Michal Privoznik mprivozn at redhat.com
Tue Apr 19 14:59:58 UTC 2016


On 19.04.2016 16:09, Christophe Fergeau wrote:
> On Tue, Apr 19, 2016 at 03:46:41PM +0200, Michal Privoznik wrote:
>> This function not only did not free return value of
>> xmlNodeListGetString() it strdup()-ed its return value. Therefore
>> plenty of memory has been lost definitely upon return from this
>> function.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  src/libvirt-php.c | 110 ++++++++++++++++++++++++++++++------------------------
>>  1 file changed, 62 insertions(+), 48 deletions(-)
>>
>> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
>> index 308e764..fb0679b 100644
>> --- a/src/libvirt-php.c
>> +++ b/src/libvirt-php.c
>> @@ -3232,19 +3232,17 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
>>      if (val != NULL) {
>>          ret = 0;
>>          for (i = 0; i < nodeset->nodeNr; i++) {
>> -            if (xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1) != NULL) {
>> -                value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1);
>> -
>> +            if ((value = xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1))) {
>>                  snprintf(key, sizeof(key), "%d", i);
>>                  VIRT_ADD_ASSOC_STRING(*val, key, value);
>> +                free(value);
>> +                value = NULL;
> 
> 'value' is an xmlChar so must be freed with xmlFree. I'd change the
> return value of get_string_from_xpath() to make it clear that xmlFree needs to
> be used to free it as well.
> 

Yeah, I could not agree more. But problem is that would be a great
patch. Moreover, xmlFree() == free() unless some debug is turned on in
libxml2 during its compilation. And since Vasiliy will make this
function go away, I'd rather push this as is.

Michal




More information about the libvir-list mailing list