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

Re: [libvirt] [PATCH] Correct two memory leaks triggered by udev events



On 05/24/2010 12:32 PM, Chris Lalancette wrote:
> On 05/24/2010 12:10 PM, Nigel Jones wrote:
>> >From 6c8183e83fbfeb031b16cf9ae2d41b16e3145378 Mon Sep 17 00:00:00 2001
>> From: Nigel Jones <dev nigelj com>
>> Date: Mon, 24 May 2010 15:05:53 +0000
>> Subject: [PATCH] Patch 2 memory leaks.
>>
>> 1. Ensure that memory is free'd from udevAddOneDevice() if the return
>>    value will be non-zero
>> 2. Release udev device reference in udevEventHandleCallback()
>>    similar to the release of the reference in udevProcessDeviceListEntry()
>> ---
>>  src/node_device/node_device_udev.c |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/node_device/node_device_udev.c
>> b/src/node_device/node_device_udev.c
>> index a1ced87..4d0effa 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -1296,6 +1296,9 @@ static int udevAddOneDevice(struct udev_device *device)
>>      ret = 0;
>>
>>  out:
>> +    if (ret != 0) {
>> +       virNodeDeviceDefFree(def); /* Free assigned memory to prevent leaks */
>> +    }
>>      return ret;
>>  }
>>
>> @@ -1426,6 +1429,7 @@ static void udevEventHandleCallback(int watch
>> ATTRIBUTE_UNUSED,
>>      }
>>
>>  out:
>> +    udev_device_unref(device);
>>      return;
>>  }
>>
> 
> Good catch on both of these.  I had to convince myself that udev_device_unref()
> would handle a NULL device (in the case of error before we allocated device), but it does.

OK, so after talking to David Allan, who originally wrote this code, it seems like
this is right approach, but there is one piece left that we have to worry about.
Namely, on a udev "change" event, we have to make sure we remove and re-add the device
to the node-device list.  Now, this *may* already be happening, but I don't quite
understand the semantics of this enough to say for sure.  I'm going to hold off
pushing this into the repo until Dave has had a chance to look at it and comment.

-- 
Chris Lalancette


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