Re: [PATCH v2 09/53] target/avr: convert to use format_state instead of dump_state

[Philippe Mathieu-Daudé]

On 9/14/21 4:19 PM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  target/avr/cpu.c | 57 ++++++++++++++++++++++++------------------------
>  1 file changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index ea14175ca5..17ff21f8be 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -145,43 +145,44 @@ static ObjectClass *avr_cpu_class_by_name(const char 
> *cpu_model)
>      return oc;
>  }
>  
> -static void avr_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> +static void avr_cpu_format_state(CPUState *cs, GString *buf, int flags)
>  {
>      AVRCPU *cpu = AVR_CPU(cs);
>      CPUAVRState *env = &cpu->env;
>      int i;
>  
> -    qemu_fprintf(f, "\n");
> -    qemu_fprintf(f, "PC:    %06x\n", env->pc_w * 2); /* PC points to words */
> -    qemu_fprintf(f, "SP:      %04x\n", env->sp);
> -    qemu_fprintf(f, "rampD:     %02x\n", env->rampD >> 16);
> -    qemu_fprintf(f, "rampX:     %02x\n", env->rampX >> 16);
> -    qemu_fprintf(f, "rampY:     %02x\n", env->rampY >> 16);
> -    qemu_fprintf(f, "rampZ:     %02x\n", env->rampZ >> 16);
> -    qemu_fprintf(f, "EIND:      %02x\n", env->eind >> 16);
> -    qemu_fprintf(f, "X:       %02x%02x\n", env->r[27], env->r[26]);
> -    qemu_fprintf(f, "Y:       %02x%02x\n", env->r[29], env->r[28]);
> -    qemu_fprintf(f, "Z:       %02x%02x\n", env->r[31], env->r[30]);
> -    qemu_fprintf(f, "SREG:    [ %c %c %c %c %c %c %c %c ]\n",
> -                 env->sregI ? 'I' : '-',
> -                 env->sregT ? 'T' : '-',
> -                 env->sregH ? 'H' : '-',
> -                 env->sregS ? 'S' : '-',
> -                 env->sregV ? 'V' : '-',
> -                 env->sregN ? '-' : 'N', /* Zf has negative logic */
> -                 env->sregZ ? 'Z' : '-',
> -                 env->sregC ? 'I' : '-');
> -    qemu_fprintf(f, "SKIP:    %02x\n", env->skip);
> -
> -    qemu_fprintf(f, "\n");
> +    g_string_append_printf(buf, "\n");

This would be g_string_append_c(buf, '\n') but in this particular case
it doesn't seem helpful so I'd directly remove it.

> +    /* PC points to words */
> +    g_string_append_printf(buf, "PC:    %06x\n", env->pc_w * 2);
> +    g_string_append_printf(buf, "SP:      %04x\n", env->sp);
> +    g_string_append_printf(buf, "rampD:     %02x\n", env->rampD >> 16);
> +    g_string_append_printf(buf, "rampX:     %02x\n", env->rampX >> 16);
> +    g_string_append_printf(buf, "rampY:     %02x\n", env->rampY >> 16);
> +    g_string_append_printf(buf, "rampZ:     %02x\n", env->rampZ >> 16);
> +    g_string_append_printf(buf, "EIND:      %02x\n", env->eind >> 16);
> +    g_string_append_printf(buf, "X:       %02x%02x\n", env->r[27], 
> env->r[26]);
> +    g_string_append_printf(buf, "Y:       %02x%02x\n", env->r[29], 
> env->r[28]);
> +    g_string_append_printf(buf, "Z:       %02x%02x\n", env->r[31], 
> env->r[30]);
> +    g_string_append_printf(buf, "SREG:    [ %c %c %c %c %c %c %c %c ]\n",
> +                           env->sregI ? 'I' : '-',
> +                           env->sregT ? 'T' : '-',
> +                           env->sregH ? 'H' : '-',
> +                           env->sregS ? 'S' : '-',
> +                           env->sregV ? 'V' : '-',
> +                           env->sregN ? '-' : 'N', /* Zf has negative logic 
> */
> +                           env->sregZ ? 'Z' : '-',
> +                           env->sregC ? 'I' : '-');
> +    g_string_append_printf(buf, "SKIP:    %02x\n", env->skip);
> +
> +    g_string_append_printf(buf, "\n");
>      for (i = 0; i < ARRAY_SIZE(env->r); i++) {
> -        qemu_fprintf(f, "R[%02d]:  %02x   ", i, env->r[i]);
> +        g_string_append_printf(buf, "R[%02d]:  %02x   ", i, env->r[i]);
>  
>          if ((i % 8) == 7) {
> -            qemu_fprintf(f, "\n");
> +            g_string_append_printf(buf, "\n");
>          }
>      }
> -    qemu_fprintf(f, "\n");
> +    g_string_append_printf(buf, "\n");

Ditto (remove).

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>  }