On Wed, May 30, 2018 at 05:35:52PM +0100, Stefan Hajnoczi wrote:
> On Tue, May 29, 2018 at 01:57:53PM +0800, Peter Xu wrote:
> > Instead, use a dynamic function to detect which clock we'll use. The
> > problem is that the old code will let monitor initialization depends on
> > qtest_enabled(). After this change, we don't have such a dependency any
> > more.
>
> There is a hidden dependency:
>
> monitor_get_clock() returns the wrong value before main() has
> processed command-line arguments.
To be more explicit:
monitor_get_clock() returns the wrong value before accelerator is
correctly setup (in configure_accelerator()).
Since the only thing that matters here is whether we're using the
qtest accelerator.
>
> Where is the guarantee that monitor_get_clock() is never called too
> early?
You are right, there is no guarantee except from programming POV.
It's only used in:
monitor_qapi_event_queue
monitor_qapi_event_handler
These two functions will never be called until accelerator is setup.
>
> At the least, monitor_get_clock() should call abort(3) if invoked too
> early. Even better would be an interface that cannot be used
> incorrectly.
Maybe then I should export the accel_initialised variable in
configure_accelerator() and then I assert with that. But that'll
further expand the series a bit.
Or, I can also mention above in the commit message to explain that a
bit.
What would you prefer?
Thanks,
--
Peter Xu
