[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 29/29] target: Use qemu_log() instead of fpri
From: |
Alistair Francis |
Subject: |
Re: [Qemu-devel] [PATCH v5 29/29] target: Use qemu_log() instead of fprintf(stderr, ...) |
Date: |
Tue, 19 Dec 2017 14:43:13 -0800 |
On Mon, Nov 13, 2017 at 11:40 PM, Thomas Huth <address@hidden> wrote:
> On 13.11.2017 23:36, Alistair Francis wrote:
>> Signed-off-by: Alistair Francis <address@hidden>
> [...]
>> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v5 29/29] target: Use qemu_log() instead of fprintf(stderr, ...),
Alistair Francis <=