Re: [Qemu-devel] [RFC] [PATCHv8 13/30] aio / timers: Add aio_timer_new wrapper

[Alex Bligh]

On 9 Aug 2013, at 15:36, Paolo Bonzini wrote:

>>> 
>>> Since we're doing a new API, I would prefer to have it as timer_init and
>>> aio_timer_init.  We can remove the allocation completely, it is a
>>> useless indirection and we misuse it since we hardly ever call
>>> qemu_free_timer.
>> 
>> Would that not require change the huge number of qemu_timer_new references
>> to use this new API? That sounds less than automatic! Not in favour of
>> that one.
> 
> qemu_timer_new can remain for now (only waiting for the next
> mass-rewriting script to be written).  I would just prefer to have the
> new AioContext-/TimerList-aware not do any allocation.

Though I've nearly completed this, I am only now (always the way)
having second thoughts about whether this is a good idea.

There are a large number of users of qemu_free_timer (now timer_free).

If someone does not call qemu_free_timer having called qemu_new_timer,
the timer sits there and basically does nothing.

If we go to the timer_init model, the timer will either be on the
stack or (more likely) inside some other struct on the heap, which
will likely have been freed. This means walking the timer list will
be dangerous.

This seems to add a good deal of fragility.

I've no objection to adding timer_init (especially as I've already
done it), but I wonder whether we're right to make this the only
interface for aio_timer etc. - i.e. maybe we should have aio_timer_new
and aio_timer_init.

WDYT?

-- 
Alex Bligh