| Message ID | d297d74fd53f91e25e4ffb980ea40d4032beaf0b.1510612380.git.alistair.francis@xilinx.com |
|---|---|
| State | New |
| Headers | show |
| Series | Remove some of the fprintf(stderr, "* | expand |
On 13.11.2017 23:36, Alistair Francis wrote: > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> [...] > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index 998fbed848..8eafe30624 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3945,11 +3945,11 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > + qemu_log("Trying to read privileged spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > if (qemu_log_separate()) { > qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); Unnecessary and wrong white space change. Please fix that line. > } > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > @@ -3962,8 +3962,8 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > + qemu_log("Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > if (qemu_log_separate()) { > qemu_log("Trying to read invalid spr %d (0x%03x) at " > TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); Now that looks wrong, too: If qemu_log_separate() returns true, the same message is printed out again? > @@ -4108,8 +4108,8 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > + qemu_log("Trying to write privileged spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > if (qemu_log_separate()) { > qemu_log("Trying to write privileged spr %d (0x%03x) at " > TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); dito > @@ -4129,8 +4129,8 @@ static void gen_mtspr(DisasContext *ctx) > qemu_log("Trying to write invalid spr %d (0x%03x) at " > TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > } > - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > + qemu_log("Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); dito > diff --git a/target/sh4/translate.c b/target/sh4/translate.c > index 703020fe87..32b72b4701 100644 > --- a/target/sh4/translate.c > +++ b/target/sh4/translate.c > @@ -427,7 +427,7 @@ static void _decode_opc(DisasContext * ctx) > } > > #if 0 > - fprintf(stderr, "Translating opcode 0x%04x\n", ctx->opcode); > + qemu_log("Translating opcode 0x%04x\n", ctx->opcode); That looks like a debug statement for the develepers ... why should that be a qemu_log() ? > #endif > > switch (ctx->opcode) { > @@ -1788,7 +1788,7 @@ static void _decode_opc(DisasContext * ctx) > break; > } > #if 0 > - fprintf(stderr, "unknown instruction 0x%04x at pc 0x%08x\n", > + qemu_log_mask(LOG_UNIMP, "unknown instruction 0x%04x at pc 0x%08x\n", > ctx->opcode, ctx->pc); Since this is now only logged conditionally if UNIMP logging is enabled, I think you should remove the "#if 0" in front of it now. > fflush(stderr); Since you don't print to stderr anymore, this fflush() also does not make sense anymore. > #endif Thomas
On Mon, Nov 13, 2017 at 11:40 PM, Thomas Huth <thuth@redhat.com> wrote: > On 13.11.2017 23:36, Alistair Francis wrote: >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > [...] >> diff --git a/target/ppc/translate.c b/target/ppc/translate.c >> index 998fbed848..8eafe30624 100644 >> --- a/target/ppc/translate.c >> +++ b/target/ppc/translate.c >> @@ -3945,11 +3945,11 @@ static inline void gen_op_mfspr(DisasContext *ctx) >> * allowing userland application to read the PVR >> */ >> if (sprn != SPR_PVR) { >> - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " >> - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> + qemu_log("Trying to read privileged spr %d (0x%03x) at " >> + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> if (qemu_log_separate()) { >> qemu_log("Trying to read privileged spr %d (0x%03x) at " >> - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > > Unnecessary and wrong white space change. Please fix that line. > >> } >> } >> gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); >> @@ -3962,8 +3962,8 @@ static inline void gen_op_mfspr(DisasContext *ctx) >> return; >> } >> /* Not defined */ >> - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " >> - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> + qemu_log("Trying to read invalid spr %d (0x%03x) at " >> + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> if (qemu_log_separate()) { >> qemu_log("Trying to read invalid spr %d (0x%03x) at " >> TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > > Now that looks wrong, too: If qemu_log_separate() returns true, the same > message is printed out again? > >> @@ -4108,8 +4108,8 @@ static void gen_mtspr(DisasContext *ctx) >> (*write_cb)(ctx, sprn, rS(ctx->opcode)); >> } else { >> /* Privilege exception */ >> - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " >> - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> + qemu_log("Trying to write privileged spr %d (0x%03x) at " >> + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> if (qemu_log_separate()) { >> qemu_log("Trying to write privileged spr %d (0x%03x) at " >> TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > > dito > >> @@ -4129,8 +4129,8 @@ static void gen_mtspr(DisasContext *ctx) >> qemu_log("Trying to write invalid spr %d (0x%03x) at " >> TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> } >> - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " >> - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); >> + qemu_log("Trying to write invalid spr %d (0x%03x) at " >> + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); > > dito > >> diff --git a/target/sh4/translate.c b/target/sh4/translate.c >> index 703020fe87..32b72b4701 100644 >> --- a/target/sh4/translate.c >> +++ b/target/sh4/translate.c >> @@ -427,7 +427,7 @@ static void _decode_opc(DisasContext * ctx) >> } >> >> #if 0 >> - fprintf(stderr, "Translating opcode 0x%04x\n", ctx->opcode); >> + qemu_log("Translating opcode 0x%04x\n", ctx->opcode); > > That looks like a debug statement for the develepers ... why should that > be a qemu_log() ? > >> #endif >> >> switch (ctx->opcode) { >> @@ -1788,7 +1788,7 @@ static void _decode_opc(DisasContext * ctx) >> break; >> } >> #if 0 >> - fprintf(stderr, "unknown instruction 0x%04x at pc 0x%08x\n", >> + qemu_log_mask(LOG_UNIMP, "unknown instruction 0x%04x at pc 0x%08x\n", >> ctx->opcode, ctx->pc); > > Since this is now only logged conditionally if UNIMP logging is enabled, > I think you should remove the "#if 0" in front of it now. > >> fflush(stderr); > > Since you don't print to stderr anymore, this fflush() also does not > make sense anymore. Ok, fixed all of these. Alistair > >> #endif > > Thomas
diff --git a/target/cris/translate.c b/target/cris/translate.c index 2831419845..d2dba748cd 100644 --- a/target/cris/translate.c +++ b/target/cris/translate.c @@ -137,7 +137,7 @@ typedef struct DisasContext { static void gen_BUG(DisasContext *dc, const char *file, int line) { - fprintf(stderr, "BUG: pc=%x %s %d\n", dc->pc, file, line); + qemu_log("BUG: pc=%x %s %d\n", dc->pc, file, line); if (qemu_log_separate()) { qemu_log("BUG: pc=%x %s %d\n", dc->pc, file, line); } diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 998fbed848..8eafe30624 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -3945,11 +3945,11 @@ static inline void gen_op_mfspr(DisasContext *ctx) * allowing userland application to read the PVR */ if (sprn != SPR_PVR) { - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); + qemu_log("Trying to read privileged spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); if (qemu_log_separate()) { qemu_log("Trying to read privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); } } gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); @@ -3962,8 +3962,8 @@ static inline void gen_op_mfspr(DisasContext *ctx) return; } /* Not defined */ - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); + qemu_log("Trying to read invalid spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); if (qemu_log_separate()) { qemu_log("Trying to read invalid spr %d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); @@ -4108,8 +4108,8 @@ static void gen_mtspr(DisasContext *ctx) (*write_cb)(ctx, sprn, rS(ctx->opcode)); } else { /* Privilege exception */ - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); + qemu_log("Trying to write privileged spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); if (qemu_log_separate()) { qemu_log("Trying to write privileged spr %d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); @@ -4129,8 +4129,8 @@ static void gen_mtspr(DisasContext *ctx) qemu_log("Trying to write invalid spr %d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); } - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); + qemu_log("Trying to write invalid spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4); /* The behaviour depends on MSR:PR and SPR# bit 0x10, @@ -7372,8 +7372,8 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) break; } if (tcg_check_temp_count()) { - fprintf(stderr, "Opcode %02x %02x %02x %02x (%08x) leaked " - "temporaries\n", opc1(ctx.opcode), opc2(ctx.opcode), + qemu_log("Opcode %02x %02x %02x %02x (%08x) leaked " + "temporaries\n", opc1(ctx.opcode), opc2(ctx.opcode), opc3(ctx.opcode), opc4(ctx.opcode), ctx.opcode); exit(1); } diff --git a/target/sh4/translate.c b/target/sh4/translate.c index 703020fe87..32b72b4701 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -427,7 +427,7 @@ static void _decode_opc(DisasContext * ctx) } #if 0 - fprintf(stderr, "Translating opcode 0x%04x\n", ctx->opcode); + qemu_log("Translating opcode 0x%04x\n", ctx->opcode); #endif switch (ctx->opcode) { @@ -1788,7 +1788,7 @@ static void _decode_opc(DisasContext * ctx) break; } #if 0 - fprintf(stderr, "unknown instruction 0x%04x at pc 0x%08x\n", + qemu_log_mask(LOG_UNIMP, "unknown instruction 0x%04x at pc 0x%08x\n", ctx->opcode, ctx->pc); fflush(stderr); #endif diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c index 384aa86027..1d4f3dd2ac 100644 --- a/target/unicore32/translate.c +++ b/target/unicore32/translate.c @@ -1936,7 +1936,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb) disas_uc32_insn(env, dc); if (num_temps) { - fprintf(stderr, "Internal resource leak before %08x\n", dc->pc); + qemu_log("Internal resource leak before %08x\n", dc->pc); num_temps = 0; }
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- target/cris/translate.c | 2 +- target/ppc/translate.c | 22 +++++++++++----------- target/sh4/translate.c | 4 ++-- target/unicore32/translate.c | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-)