On Thu, Dec 14, 2017 at 2:46 PM, Alistair Francis <alistair.fran...@xilinx.com> wrote: > On Wed, Dec 13, 2017 at 11:58 AM, Philippe Mathieu-Daudé > <f4...@amsat.org> wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4...@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.