qemu-trivial.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: Alistair Francis <alistair.francis@xilinx.com>,
	qemu-devel@nongnu.org, qemu-trivial@nongnu.org
Cc: alistair23@gmail.com, armbru@redhat.com, qemu-ppc@nongnu.org
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH v5 29/29] target: Use qemu_log() instead of fprintf(stderr, ...)
Date: Tue, 14 Nov 2017 08:40:43 +0100	[thread overview]
Message-ID: <7a185bec-43b3-7fcd-d419-9af143b2e3f9@redhat.com> (raw)
In-Reply-To: <d297d74fd53f91e25e4ffb980ea40d4032beaf0b.1510612380.git.alistair.francis@xilinx.com>

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


  reply	other threads:[~2017-11-14  7:41 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-13 22:34 [Qemu-trivial] [PATCH v5 00/29] Remove some of the fprintf(stderr, "* Alistair Francis
2017-11-13 22:34 ` [Qemu-trivial] [PATCH v5 01/29] audio: Replace AUDIO_FUNC with __func__ Alistair Francis
2017-11-14  9:55   ` Gerd Hoffmann
2017-11-13 22:34 ` [Qemu-trivial] [PATCH v5 02/29] Replace all occurances of __FUNCTION__ " Alistair Francis
2017-11-14  9:56   ` Gerd Hoffmann
2017-11-13 22:34 ` [Qemu-trivial] [PATCH v5 03/29] Fixes after renaming __FUNCTION__ to __func__ Alistair Francis
2017-11-13 22:34 ` [Qemu-trivial] [PATCH v5 04/29] hw/arm: Replace fprintf(stderr, "*\n" with error_report() Alistair Francis
2017-11-13 22:34 ` [Qemu-trivial] [PATCH v5 05/29] hw/dma: " Alistair Francis
2017-11-13 22:34 ` [Qemu-trivial] [PATCH v5 06/29] hw/gpio: " Alistair Francis
2017-11-13 22:34 ` [Qemu-trivial] [PATCH v5 07/29] hw/i2c: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 09/29] hw/ide: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 10/29] hw/intc: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 11/29] hw/ipmi: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 12/29] hw/isa: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 13/29] hw/lm32: " Alistair Francis
2017-11-14  6:54   ` Michael Walle
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 14/29] hw/mips: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 15/29] hw/moxie: " Alistair Francis
2017-11-14  7:53   ` [Qemu-trivial] [Qemu-devel] " Thomas Huth
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 16/29] hw/nios2: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 17/29] hw/nvram: " Alistair Francis
2017-11-13 23:56   ` Philippe Mathieu-Daudé
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 18/29] hw/openrisc: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 19/29] hw/pci*: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 20/29] hw/ppc: " Alistair Francis
2017-11-14  8:30   ` [Qemu-trivial] [Qemu-ppc] " Thomas Huth
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 21/29] hw/s390x: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 22/29] hw/scsi: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 23/29] hw/sd: " Alistair Francis
2017-11-13 22:35 ` [Qemu-trivial] [PATCH v5 24/29] hw/sparc*: " Alistair Francis
2017-11-17 13:45   ` Mark Cave-Ayland
2017-11-13 22:36 ` [Qemu-trivial] [PATCH v5 25/29] hw/ssi: " Alistair Francis
2017-11-13 22:36 ` [Qemu-trivial] [PATCH v5 26/29] hw/timer: " Alistair Francis
2017-11-14  7:51   ` [Qemu-trivial] [Qemu-devel] " Thomas Huth
2017-11-13 22:36 ` [Qemu-trivial] [PATCH v5 27/29] hw/xen*: " Alistair Francis
2017-11-13 22:36 ` [Qemu-trivial] [PATCH v5 28/29] tcg: " Alistair Francis
2017-11-14  7:44   ` [Qemu-trivial] [Qemu-devel] " Thomas Huth
2017-11-13 22:36 ` [Qemu-trivial] [PATCH v5 29/29] target: Use qemu_log() instead of fprintf(stderr, ...) Alistair Francis
2017-11-14  7:40   ` Thomas Huth [this message]
2017-12-19 22:43     ` [Qemu-trivial] [Qemu-devel] " Alistair Francis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a185bec-43b3-7fcd-d419-9af143b2e3f9@redhat.com \
    --to=thuth@redhat.com \
    --cc=alistair.francis@xilinx.com \
    --cc=alistair23@gmail.com \
    --cc=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).