[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] monitor: Split mon_get_cpu fn to remove
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] monitor: Split mon_get_cpu fn to remove ENV_GET_CPU |
Date: |
Tue, 16 Jun 2015 17:09:00 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Peter Crosthwaite <address@hidden> writes:
> The monitor currently has one helper, mon_get_cpu() which will return
> an env pointer. The target specific users of this API want an env, but
> all the target agnostic users really just want the cpu pointer. These
> users then need to use the target-specifically defined ENV_GET_CPU to
> navigate back up to the CPU from the ENV. Split the API for the two
> uses cases to remove all need for ENV_GET_CPU.
>
> Reviewed-by: Richard Henderson <address@hidden>
> Reviewed-by: Andreas Färber <address@hidden>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
> Changed since v1:
> s/mon_get_env/mon_get_cpu_env (Andreas review)
> Avoid C99 declaration (RH review)
> ---
> monitor.c | 65
> ++++++++++++++++++++++++++++-----------------------------------
> 1 file changed, 29 insertions(+), 36 deletions(-)
>
[...]
> @@ -1208,16 +1203,14 @@ static void monitor_printc(Monitor *mon, int c)
> static void memory_dump(Monitor *mon, int count, int format, int wsize,
> hwaddr addr, int is_physical)
> {
> - CPUArchState *env;
> int l, line_size, i, max_digits, len;
> uint8_t buf[16];
> uint64_t v;
>
> if (format == 'i') {
> - int flags;
> - flags = 0;
> - env = mon_get_cpu();
> + int flags = 0;
> #ifdef TARGET_I386
> + CPUArchState *env = mon_get_cpu_env();
> if (wsize == 2) {
> flags = 1;
> } else if (wsize == 4) {
> @@ -1238,10 +1231,11 @@ static void memory_dump(Monitor *mon, int count, int
> format, int wsize,
> }
> #endif
> #ifdef TARGET_PPC
> + CPUArchState *env = mon_get_cpu_env();
> flags = msr_le << 16;
> flags |= env->bfd_mach;
> #endif
> - monitor_disas(mon, env, addr, count, is_physical, flags);
> + monitor_disas(mon, mon_get_cpu_env(), addr, count, is_physical,
> flags);
> return;
> }
>
> @@ -1280,8 +1274,7 @@ static void memory_dump(Monitor *mon, int count, int
> format, int wsize,
> if (is_physical) {
> cpu_physical_memory_read(addr, buf, l);
> } else {
> - env = mon_get_cpu();
> - if (cpu_memory_rw_debug(ENV_GET_CPU(env), addr, buf, l, 0) < 0) {
> + if (cpu_memory_rw_debug(mon_get_cpu(), addr, buf, l, 0) < 0) {
> monitor_printf(mon, " Cannot access memory\n");
> break;
> }
You call mon_get_cpu_env() and mon_get_cpu() multiple times in this
function, and declare CPUArchState *env twice (which rang my shadowing
alarm bell; fortunately the two are under mutually exclusive #ifdef).
Why not simply do
CPUState *cpu = mon_get_cpu();
CPUArchState *env = mon_get_cpu_env();
right at the beginning and be done with it?
Not a big deal, I'm willing to take this patch through my tree as is.
[...]