On Thu, Jul 25, 2013 at 10:46:18AM +0100, Alex Bligh wrote: > >>@@ -61,6 +71,15 @@ int64_t cpu_get_ticks(void); > >> void cpu_enable_ticks(void); > >> void cpu_disable_ticks(void); > >> > >>+static inline int64_t qemu_soonest_timeout(int64_t timeout1, int64_t > >>timeout2) +{ > >>+ /* we can abuse the fact that -1 (which means infinite) is a maximal > >>+ * value when cast to unsigned. As this is disgusting, it's kept in > >>+ * one inline function. > >>+ */ > >>+ return ((uint64_t) timeout1 < (uint64_t) timeout2) ? timeout1 : > >>timeout2; > > > >The straightforward version isn't much longer than the commented casting > >trick: > > > >if (timeout1 == -1) { > > return timeout2; > >} else if (timeout2 == -1) { > > return timeout1; > >} else { > > return timeout1 < timeout2 ? timeout1 : timeout2; > >} > > Well, it should be (timeout1 < 0) for consistency. It may be a micro > optimisation but I'm pretty sure the casting trick will produce better > code. With the comment, it's arguably more readable too.
Seems like a compiler could be smart enough to use unsigned instructions. Seems like a ">> 9" vs "/ 512" micro-optimization to me. > >>+void qemu_free_clock(QEMUClock *clock) > >>+{ > >>+ QLIST_REMOVE(clock, list); > >>+ g_free(clock); > > > >assert that there are no timers left? > > Yes I wasn't quite sure of the right semantics here as no clocks are > currently ever destroyed. I'm not quite sure how we know all timers > are destroyed when an AioContext is destroyed. Should I go and manually > free them or assert the right way? It is not possible to free them since their owner still holds a pointer. That is why I'd use an assert. The code needs to be written so that timers are destroyed before the clock is destroyed. Stefan