On 02/07/2011 09:29 AM, Avi Kivity wrote:
On 02/07/2011 05:17 PM, Jan Kiszka wrote:
On 2011-02-07 16:13, Avi Kivity wrote:
> On 02/07/2011 05:01 PM, Anthony Liguori wrote:
>>
>> typedef struct PeriodicTimer PeriodicTimer;
>>
>> /**
>> * @accumulated_ticks: the number of unacknowledged ticks in total
>> since the creation of the timer
>> **/
>
> Outdated comment even before the code is committed. Will be hard
to beat.
>
>> typedef void (PeriodicTimerFunc)(void *opaque);
>
> s/void *opaque/PeriodicTimer *timer/
>
> Down with opaques!
What else? DeviceState?
typedef void (PeriodicTimerFunc)(PeriodicTimer *timer);
the callback then uses container_of() to get whatever its internal
data structure is from the embedded PeriodicTimer.
For the purposes of this, I think passing an opaque is better because
the signature stays the same as the existing timer callback. That makes
conversion a bit friendlier.
I think it's better to avoid introducing stylistic changes with new
interfaces. I think they should be done separately.
Regards,
Anthony Liguori
>>
>> PeriodicTimer *periodic_timer_new(PeriodicTimerFunc *cb, void
*opaque);
>>
>
> void periodic_timer_init(PeriodicTimer *timer, PeriodicTimerFunc
*cb);
>
> It is better to embed than to reference.
Likely, though this diverges from exiting QEMUTimer.
That's the more modern style. Saves allocations and dereferences, and
is more type safe.
