Message ID | 20171213195852.30439-10-f4bug@amsat.org |
---|---|
State | Superseded, archived |
Headers | show |
Series | SDHCI housekeeping | expand |
On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/sd/sdhci.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index 893be0e606..044e3d62f1 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -31,6 +31,7 @@ > #include "hw/sd/sdhci.h" > #include "sdhci-internal.h" > #include "sd-internal.h" > +#include "qapi/error.h" > #include "qemu/log.h" > > /* host controller debug messages */ > @@ -1191,17 +1192,21 @@ 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); > + s->fifo_buffer = NULL; I don't think setting this to NULL is required. Shouldn't g_free() do that for you? Alistair > +} > + > 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); > + > qemu_free_irq(s->eject_cb); > qemu_free_irq(s->ro_cb); > - > - g_free(s->fifo_buffer); > - s->fifo_buffer = NULL; > } > > static bool sdhci_pending_insert_vmstate_needed(void *opaque) > @@ -1302,6 +1307,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); > } > > @@ -1355,11 +1362,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 > >
On Thu, Dec 14, 2017 at 2:46 PM, Alistair Francis <alistair.francis@xilinx.com> wrote: > On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé > <f4bug@amsat.org> wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/sd/sdhci.c | 21 ++++++++++++++++++--- >> 1 file changed, 18 insertions(+), 3 deletions(-) >> >> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c >> index 893be0e606..044e3d62f1 100644 >> --- a/hw/sd/sdhci.c >> +++ b/hw/sd/sdhci.c >> @@ -31,6 +31,7 @@ >> #include "hw/sd/sdhci.h" >> #include "sdhci-internal.h" >> #include "sd-internal.h" >> +#include "qapi/error.h" >> #include "qemu/log.h" >> >> /* host controller debug messages */ >> @@ -1191,17 +1192,21 @@ 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); >> + s->fifo_buffer = NULL; > > I don't think setting this to NULL is required. Shouldn't g_free() do > that for you? You are right, good eye :) I didn't notice, this was just code movement from the current code. Indeed g_free() doesn't free() if the pointer is NULL, however since it doesn't have access to the pointer address it can't set it to NULL. In some case it matters (RCU code maybe?) but I just checked and it this case it doesn't, so I'll drop this line.
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 893be0e606..044e3d62f1 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -31,6 +31,7 @@ #include "hw/sd/sdhci.h" #include "sdhci-internal.h" #include "sd-internal.h" +#include "qapi/error.h" #include "qemu/log.h" /* host controller debug messages */ @@ -1191,17 +1192,21 @@ 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); + s->fifo_buffer = NULL; +} + 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); + qemu_free_irq(s->eject_cb); qemu_free_irq(s->ro_cb); - - g_free(s->fifo_buffer); - s->fifo_buffer = NULL; } static bool sdhci_pending_insert_vmstate_needed(void *opaque) @@ -1302,6 +1307,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); } @@ -1355,11 +1362,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); }
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/sd/sdhci.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)