[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] hw/sd/sd: Delay timer_new_ns() from init to realize to a
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 2/2] hw/sd/sd: Delay timer_new_ns() from init to realize to avoid memleaks |
Date: |
Fri, 5 Jun 2020 07:05:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 2/17/20 2:26 PM, Peter Maydell wrote:
> On Sat, 15 Feb 2020 at 15:48, Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
>>
>> In commit f3a508eb4e the Euler Robot reported calling timer_new()
>> in instance_init() can leak heap memory. The easier fix is to
>> delay the timer creation at instance realize(). Similarly move
>> timer_del() into a new instance unrealize() method.
>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 71a9af09ab..d72cf3de2a 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -2058,14 +2058,12 @@ static void sd_instance_init(Object *obj)
>> SDState *sd = SD_CARD(obj);
>>
>> sd->enable = true;
>> - sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup,
>> sd);
>> }
>>
>> static void sd_instance_finalize(Object *obj)
>> {
>> SDState *sd = SD_CARD(obj);
>>
>> - timer_del(sd->ocr_power_timer);
>> timer_free(sd->ocr_power_timer);
>> }
>>
>> @@ -2098,6 +2096,15 @@ static void sd_realize(DeviceState *dev, Error **errp)
>> }
>> blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
>> }
>> +
>> + sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup,
>> sd);
>> +}
>> +
>> +static void sd_unrealize(DeviceState *dev, Error **errp)
>> +{
>> + SDState *sd = SD_CARD(dev);
>> +
>> + timer_del(sd->ocr_power_timer);
>> }
>
> Here too the old code was doing things correctly in that
> it does a timer_del/timer_free on the timer it allocates
> in instance_init, and the new code has weirdly split the
> freeing between unrealize and finalize.
Indeed I now see it, thanks.
>
> thanks
> -- PMM
>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH 2/2] hw/sd/sd: Delay timer_new_ns() from init to realize to avoid memleaks,
Philippe Mathieu-Daudé <=