Re: [Qemu-devel] [PATCH RFC qom-cpu 01/41] log: Change log_cpu_state[_mask]() argument to CPUState

["Andreas Färber" <afaerber@suse.de>]

Am 01.07.2013 19:03, schrieb Richard Henderson:
> On 06/29/2013 01:01 PM, Andreas Färber wrote:
>>          if ((env->cr[0] & CR0_PE_MASK)) {
>> +            X86CPU *cpu = x86_env_get_cpu(env);
>>              static int count;
>>  
>>              qemu_log("%6d: v=%02x e=%04x i=%d cpl=%d IP=%04x:" TARGET_FMT_lx
>> @@ -1180,7 +1181,7 @@ static void do_interrupt_all(CPUX86State *env, int intno, int is_int,
>>                  qemu_log(" env->regs[R_EAX]=" TARGET_FMT_lx, env->regs[R_EAX]);
>>              }
>>              qemu_log("\n");
>> -            log_cpu_state(env, CPU_DUMP_CCOP);
>> +            log_cpu_state(CPU(cpu), CPU_DUMP_CCOP);
> 
> Not a bug, but I'd like to know your rationale for adding X86CPU *cpu variables
> as opposed to CPUState *cs variables?  Especially when the cpu variable is
> never used without the cast to CPU.

It's been a few days already, but I believe that I was preparing for
changing the function argument to X86CPU for any static helpers. So the
local variable would get dropped or replaced by a CPUX86State variable.

Generally, Anthony had veto'ed against CPU(cpu)->something, so I
introduce CPUState variables to avoid that, but if there's only one use
case like here and we stay within 80 chars then there is no strict need
for a separate variable IMO, whether FooCPU or CPUState.
Similarly, when there's just one or two short uses of CPU*State I have
chosen to not introduce an extra CPUFooState variable (e.g., set_pc).

> Otherwise,
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks. If Blue or you don't have any further comments on the previous
part 10 series v3, I will queue that on qom-cpu (rebasing right now).

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg