On 9/15/21 10:49 AM, Daniel P. Berrangé wrote: > On Wed, Sep 15, 2021 at 09:13:14AM +0200, Philippe Mathieu-Daudé wrote: >> On 9/14/21 4:19 PM, Daniel P. Berrangé wrote: >>> Signed-off-by: Daniel P. Berrangé <berra...@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. > > I don't want to change output format of the commands, with exception of > error reporting, as this is intended to be just refactoring patch, not > a cleanup patch.
I misread cpu_dump_state(), I thought it was already appending a newline, but it is not: @@ -106,6 +106,21 @@ void cpu_dump_state(CPUState *cpu, FILE *f, int flags) if (cc->dump_state) { cpu_synchronize_state(cpu); cc->dump_state(cpu, f, flags); + } else if (cc->format_state) { + g_autoptr(GString) buf = g_string_new(""); + cpu_synchronize_state(cpu); + cc->format_state(cpu, buf, flags); + qemu_fprintf(f, "%s", buf->str); So we are fine. > I'm not convinced it is worth special casing single byte strings to > use g_string_append_c either really. OK. R-b stands ;)