On 16 December 2014 at 16:59, Laszlo Ersek <ler...@redhat.com> 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