From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39333)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1UtiNI-00067g-Ki
	for qemu-devel@nongnu.org; Mon, 01 Jul 2013 13:58:53 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1Uti4W-0005H4-TK
	for qemu-devel@nongnu.org; Mon, 01 Jul 2013 13:39:30 -0400
Message-ID: <51D1BA84.6080502@suse.de>
Date: Mon, 01 Jul 2013 19:21:08 +0200
From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= <afaerber@suse.de>
MIME-Version: 1.0
References: <1372536117-28167-1-git-send-email-afaerber@suse.de>
	<1372536117-28167-2-git-send-email-afaerber@suse.de>
	<51D1B647.1060806@twiddle.net>
In-Reply-To: <51D1B647.1060806@twiddle.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH RFC qom-cpu 01/41] log: Change
 log_cpu_state[_mask]() argument to CPUState
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Richard Henderson <rth@twiddle.net>
Cc: Peter Maydell <peter.maydell@linaro.org>, Anthony Green <green@moxielogic.com>, Riku Voipio <riku.voipio@iki.fi>, qemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>, Blue Swirl <blauwirbel@gmail.com>, Michael Walle <michael@walle.cc>, qemu-ppc <qemu-ppc@nongnu.org>, Paul Brook <paul@codesourcery.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Jan Kiszka <jan.kiszka@web.de>, Aurelien Jarno <aurelien@aurel32.net>

Am 01.07.2013 19:03, schrieb Richard Henderson:
> On 06/29/2013 01:01 PM, Andreas F=C3=A4rber wrote:
>>          if ((env->cr[0] & CR0_PE_MASK)) {
>> +            X86CPU *cpu =3D x86_env_get_cpu(env);
>>              static int count;
>> =20
>>              qemu_log("%6d: v=3D%02x e=3D%04x i=3D%d cpl=3D%d IP=3D%04=
x:" TARGET_FMT_lx
>> @@ -1180,7 +1181,7 @@ static void do_interrupt_all(CPUX86State *env, i=
nt intno, int is_int,
>>                  qemu_log(" env->regs[R_EAX]=3D" TARGET_FMT_lx, env->r=
egs[R_EAX]);
>>              }
>>              qemu_log("\n");
>> -            log_cpu_state(env, CPU_DUMP_CCOP);
>> +            log_cpu_state(CPU(cpu), CPU_DUMP_CCOP);
>=20
> Not a bug, but I'd like to know your rationale for adding X86CPU *cpu v=
ariables
> 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,
>=20
> 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

--=20
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC=
rnberg