[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 07/42] sdhci: refactor common sysbus/pci unre
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v3 07/42] sdhci: refactor common sysbus/pci unrealize() into sdhci_unrealizefn() |
Date: |
Wed, 3 Jan 2018 07:10:10 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
Hi Fam,
On 01/03/2018 04:56 AM, Fam Zheng wrote:
> On Fri, 12/29 14:48, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>> hw/sd/sdhci.c | 19 ++++++++++++++++---
>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>> index ad5853d527..06a1ec6f91 100644
>> --- a/hw/sd/sdhci.c
>> +++ b/hw/sd/sdhci.c
>> @@ -31,6 +31,7 @@
>> #include "qemu/bitops.h"
>> #include "hw/sd/sdhci.h"
>> #include "sdhci-internal.h"
>> +#include "qapi/error.h"
>> #include "qemu/log.h"
>>
>> /* host controller debug messages */
>> @@ -1203,15 +1204,17 @@ static void sdhci_realizefn(SDHCIState *s, Error
>> **errp)
>> SDHC_REGISTERS_MAP_SIZE);
>> }
>>
>> +static void sdhci_unrealizefn(SDHCIState *s, Error **errp)
>> +{
>> + g_free(s->fifo_buffer);
>
> Set s->fifo_buffer to NULL to avoid double-free and/or use-after-free?
> Especially since you call this from both the ->exit and the ->unrealize
> callbacks.
Yes, I agree this would be safer. I'll also add a comment around.
Thanks!
>> +}
>> +
>> static void sdhci_uninitfn(SDHCIState *s)
>> {
>> timer_del(s->insert_timer);
>> timer_free(s->insert_timer);
>> timer_del(s->transfer_timer);
>> timer_free(s->transfer_timer);
>> -
>> - g_free(s->fifo_buffer);
>> - s->fifo_buffer = NULL;
>> }
>>
>> static bool sdhci_pending_insert_vmstate_needed(void *opaque)
>> @@ -1312,6 +1315,8 @@ static void sdhci_pci_realize(PCIDevice *dev, Error
>> **errp)
>> static void sdhci_pci_exit(PCIDevice *dev)
>> {
>> SDHCIState *s = PCI_SDHCI(dev);
>> +
>> + sdhci_unrealizefn(s, &error_abort);
>> sdhci_uninitfn(s);
>> }
>>
>> @@ -1365,11 +1370,19 @@ static void sdhci_sysbus_realize(DeviceState *dev,
>> Error ** errp)
>> sysbus_init_mmio(sbd, &s->iomem);
>> }
>>
>> +static void sdhci_sysbus_unrealize(DeviceState *dev, Error **errp)
>> +{
>> + SDHCIState *s = SYSBUS_SDHCI(dev);
>> +
>> + sdhci_unrealizefn(s, errp);
>> +}
>> +
>> static void sdhci_sysbus_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *dc = DEVICE_CLASS(klass);
>>
>> dc->realize = sdhci_sysbus_realize;
>> + dc->unrealize = sdhci_sysbus_unrealize;
>>
>> sdhci_class_init(klass, data);
>> }
>> --
>> 2.15.1
>>
>>
>
> Fam
>