Patchwork hw/smbus_eeprom.c : fix memory leak

login
register
mail settings
Submitter Zhi Hui Li
Date Dec. 2, 2011, 3:23 a.m.
Message ID <1322796212-324-1-git-send-email-zhihuili@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/128795/
State New
Headers show

Comments

Zhi Hui Li - Dec. 2, 2011, 3:23 a.m.
Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
---
 hw/smbus_eeprom.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Mark Wu - Dec. 2, 2011, 3:47 a.m.
On 12/02/2011 11:23 AM, Li Zhi Hui wrote:
> Signed-off-by: Li Zhi Hui<zhihuili@linux.vnet.ibm.com>
> ---
>   hw/smbus_eeprom.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/hw/smbus_eeprom.c b/hw/smbus_eeprom.c
> index 5d080ab..d66fbbc 100644
> --- a/hw/smbus_eeprom.c
> +++ b/hw/smbus_eeprom.c
> @@ -142,4 +142,5 @@ void smbus_eeprom_init(i2c_bus *smbus, int nb_eeprom,
>           qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
>           qdev_init_nofail(eeprom);
>       }
> +    g_free(eeprom_buf);
>   }
The memory pointed by eeprom_buf is used as 'data' buf of 8 
SMBusEEPROMDevice after initialization.  And it calls qdev_init_nofail, 
which will exit on failure. it should be ok to leave it there. Anyway, 
you shouldn't free it in the initialization.

Mark.
Zhi Hui Li - Dec. 2, 2011, 5:41 a.m.
On 2011年12月02日 11:47, Mark Wu wrote:
> On 12/02/2011 11:23 AM, Li Zhi Hui wrote:
>> Signed-off-by: Li Zhi Hui<zhihuili@linux.vnet.ibm.com>
>> ---
>> hw/smbus_eeprom.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/smbus_eeprom.c b/hw/smbus_eeprom.c
>> index 5d080ab..d66fbbc 100644
>> --- a/hw/smbus_eeprom.c
>> +++ b/hw/smbus_eeprom.c
>> @@ -142,4 +142,5 @@ void smbus_eeprom_init(i2c_bus *smbus, int nb_eeprom,
>> qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
>> qdev_init_nofail(eeprom);
>> }
>> + g_free(eeprom_buf);
>> }
> The memory pointed by eeprom_buf is used as 'data' buf of 8
> SMBusEEPROMDevice after initialization. And it calls qdev_init_nofail,
> which will exit on failure. it should be ok to leave it there. Anyway,
> you shouldn't free it in the initialization.
>
> Mark.
>
>
I'm afraid I was too fast when I sent a patch with memory leak, and 
don't look it carefully , It was a pointer to a pointer.

I agree with Mark Wu.

Patch

diff --git a/hw/smbus_eeprom.c b/hw/smbus_eeprom.c
index 5d080ab..d66fbbc 100644
--- a/hw/smbus_eeprom.c
+++ b/hw/smbus_eeprom.c
@@ -142,4 +142,5 @@  void smbus_eeprom_init(i2c_bus *smbus, int nb_eeprom,
         qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
         qdev_init_nofail(eeprom);
     }
+    g_free(eeprom_buf);
 }