[Qemu-devel] Re: [PATCH -v2 1/2] Log reset events

[Jan Kiszka <jan.kiszka@siemens.com>]

Jan Kiszka wrote:
> Kevin Wolf wrote:
>> Laurent Desnogues schrieb:
>>> On Fri, Jan 16, 2009 at 1:14 PM, Kevin Wolf <kwolf@suse.de> wrote:
>>> [..]
>>>>> @@ -159,6 +159,12 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
>>>>>  void cpu_reset(CPUARMState *env)
>>>>>  {
>>>>>      uint32_t id;
>>>>> +
>>>>> +    if (qemu_loglevel_mask(CPU_LOG_RESET)) {
>>>>> +        fprintf(logfile, "CPU Reset (CPU %d)\n", env->cpu_index);
>>>>> +        cpu_dump_state(env, logfile, fprintf, 0);
>>>> Shouldn't that be qemu_log() and log_cpu_state() now?
>>> Yes. Sorry Jan :)
> 
> Well, the patch description misled me a bit ("global s/loglevel &
> X/qemu_loglevel_mask(X)/"), but I also could have checked more carefully
> what actually changed.
> 
>>> A trick to check things are done properly is to make sure you
>>> don't ever use logfile or loglevel variables.
>> Maybe qemu_log() should be a function rather than a macro? Then loglevel
>> and logfile could be static and the build would break if you try to use
>> them outside the logging code.
> 
> Sounds good (function-like macros are for corner cases only).
> 
> I will try to make logfile/loglevel static and fix my patch accordingly.

Making logfile and loglevel static means enforcing a function call just
to check if we perform logging. Would the additional overhead be
acceptable for all qemu_log* spots? Unless maintainers tell me "yes", I
will not touch this part for now.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 26
Corporate Competence Center Embedded Linux