Re: [Qemu-devel] [PATCH 08/14] trace: Support for dynamically enabling/disabling trace events.

[Stefan Hajnoczi]

On Thu, Aug 12, 2010 at 7:02 PM, Blue Swirl <address@hidden> wrote:
> On Thu, Aug 12, 2010 at 10:36 AM, Stefan Hajnoczi
> <address@hidden> wrote:
>> From: Prerna Saxena <address@hidden>
>> +#if defined(CONFIG_SIMPLE_TRACE)
>> +static void do_info_trace(Monitor *mon)
>> +{
>> +    st_print_trace((FILE *)mon, &monitor_fprintf);
>
> The cast is ugly. Is there no other way than passing falsified types?

Yes.  This approach is the solution that monitor.c currently uses for
decoupling subsystem code from monitor code:

static void do_info_registers(Monitor *mon)
{
    CPUState *env;
    env = mon_get_cpu();
#ifdef TARGET_I386
    cpu_dump_state(env, (FILE *)mon, monitor_fprintf,
                   X86_DUMP_FPU);
#else
    cpu_dump_state(env, (FILE *)mon, monitor_fprintf,
                   0);
#endif
}

...and others do it in order to avoid telling subsystems about
Monitor.  There are 4 other places in monitor.c before this patch that
use this technique.  I suggest leaving it and if someone implements a
better approach, a search-replace will update monitor.c in one patch.

>> diff --git a/simpletrace.c b/simpletrace.c
>> index a6afc51..311fa44 100644
>> --- a/simpletrace.c
>> +++ b/simpletrace.c
>> @@ -11,6 +11,7 @@
>>  #include <stdlib.h>
>>  #include <stdint.h>
>>  #include <stdio.h>
>> +#include <time.h>
>
> The changes below don't seem to require this addition.

You are right, this is a rebase/squash issue.  Sorry, will fix in v2.

Stefan