Re: [Qemu-devel] [PATCH v4 3/8] fw_cfg: introduce the "data_memwidth" property

[Peter Maydell]

On 16 December 2014 at 16:59, Laszlo Ersek <address@hidden> wrote:
> To elaborate on the above -- the fw_cfg device appears to be
> undestructible at the moment. It has no unrealize callback. If it were
> destructible, then the above leak would be the smallest of concerns --
> it doesn't unmap nor destroy the memory regions that implement the
> various registers.
>
> So, I think the above is not an actual leak, because the result of
> g_memdup() can never become unreferenced.

True, and we have a lot of device that are in this same
category of "can't ever be destroyed". However it is setting
up a minor beartrap for ourselves in future if we have
allocations which aren't tracked via a field in the device's
state structure, because the obvious future implementation of
destruction for a device is "just free/destroy everything
that is in the state struct".

NB: I think what Alex had in mind with his option (2) was
just to have a "MemoryRegionOps ops;" field in the state
struct, and then use "s->ops = data_mem_ops;" rather than
the memdup. That retains the use of the static field for
the non-variable-width case, it's just that instead of
allocating off the heap for the var-width setup we use
an inline lump of memory in a struct we're already allocing.

I don't think I care very much about this, but Alex's
suggestion 2 is slightly nicer I guess. Adding a whole
unrealize callback is definitely vastly overkill.

-- PMM