Re: [Qemu-devel] [PATCH v1 8/8] target: Replace fprintf(stderr, "*\n" with error_report()
On 09/25/2017 05:09 PM, Alistair Francis wrote:
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 38a999e6f1..8847005984 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -138,7 +138,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);
> +error_report("BUG: pc=%x %s %d", dc->pc, file, line);
> if (qemu_log_separate()) {
> qemu_log("BUG: pc=%x %s %d\n", dc->pc, file, line);
> }
Eh, no, this should simply use qemu_log.
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 606b605ba0..a3da22580e 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -3943,8 +3943,8 @@ 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);
> +error_report("Trying to read privileged spr %d (0x%03x) at "
> +TARGET_FMT_lx "", 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);
Likewise. And all the others.
Perhaps we should kill qemu_log_separate as a bad idea?
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 10191073b2..9f95410e53 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -438,7 +438,7 @@ static void _decode_opc(DisasContext * ctx)
> }
>
> #if 0
> -fprintf(stderr, "Translating opcode 0x%04x\n", ctx->opcode);
> +error_report("Translating opcode 0x%04x", ctx->opcode);
> #endif
qemu_log.
>
> switch (ctx->opcode) {
> @@ -1799,7 +1799,7 @@ static void _decode_opc(DisasContext * ctx)
> break;
> }
> #if 0
> -fprintf(stderr, "unknown instruction 0x%04x at pc 0x%08x\n",
> +error_report("unknown instruction 0x%04x at pc 0x%08x",
> ctx->opcode, ctx->pc);
> fflush(stderr);
> #endif
qemu_log_mask(LOG_UNIMP, ...)
> @@ -1940,7 +1940,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);
> +error_report("Internal resource leak before %08x", dc->pc);
> num_temps = 0;
> }
>
>
qemu_log.
r~
Re: [Qemu-devel] [PATCH v1 8/8] target: Replace fprintf(stderr, "*\n" with error_report()
On 26.09.2017 02:09, Alistair Francis wrote:
> Replace a large number of the fprintf(stderr, "*\n" calls with
> error_report(). The functions were renamed with these commands and then
> compiler issues where manually fixed.
[...]
> @@ -9281,18 +9281,19 @@ static void init_ppc_proc(PowerPCCPU *cpu)
> case POWERPC_FLAG_PMM:
> break;
> default:
> -fprintf(stderr, "PowerPC MSR definition inconsistency\n"
> -"Should define POWERPC_FLAG_PX or POWERPC_FLAG_PMM\n");
> +error_report("PowerPC MSR definition inconsistency");
> +error_printf("Should define POWERPC_FLAG_PX or
> POWERPC_FLAG_PMM");
> exit(1);
> }
> } else if (env->flags & (POWERPC_FLAG_PX | POWERPC_FLAG_PMM)) {
> -fprintf(stderr, "PowerPC MSR definition inconsistency\n"
> -"Should not define POWERPC_FLAG_PX nor POWERPC_FLAG_PMM\n");
> +error_report("PowerPC MSR definition inconsistency");
> +error_printf("Should not define POWERPC_FLAG_PX nor
> POWERPC_FLAG_PMM");
> exit(1);
> }
> if ((env->flags & (POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_BUS_CLK)) == 0) {
> -fprintf(stderr, "PowerPC flags inconsistency\n"
> -"Should define the time-base and decrementer clock
> source\n");
> +error_report("PowerPC MSR definition inconsistency");
> +error_printf("Should define the time-base and decrementer clock"
> + " source");
> exit(1);
> }
Replacing fprintf with error_report+error_printf is kind of very ugly.
While error_report adds newline to the output, error_printf does not (as
far as I can see) ... did you check how such a message looks like when
it is generated during runtime?
Also, the strings belong together, so printing them with two function
calls sound wrong to me... maybe they should be joined without newline
in the middle instead?
Thomas
[Qemu-devel] [PATCH v1 8/8] target: Replace fprintf(stderr, "*\n" with error_report()
Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr,
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr,
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr,
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr,
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr,
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}'
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
Some lines where then manually tweaked to pass checkpatch.
Signed-off-by: Alistair Francis
Cc: "Edgar E. Iglesias"
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: Marcelo Tosatti
Cc: Michael Walle
Cc: Aurelien Jarno
Cc: Yongbok Kim
Cc: Christian Borntraeger
Cc: Cornelia Huck
Cc: Guan Xuetao
Cc: qemu-...@nongnu.org
Cc: qemu-...@nongnu.org
---
target/arm/arm-powerctl.c| 5 +++--
target/arm/arm-semi.c| 3 ++-
target/arm/helper.c | 4 ++--
target/arm/kvm.c | 16 ++---
target/arm/kvm32.c | 2 +-
target/arm/kvm64.c | 2 +-
target/arm/translate-a64.c | 4 ++--
target/arm/translate.c | 2 +-
target/cris/helper.c | 2 +-
target/cris/translate.c | 2 +-
target/i386/hax-all.c| 52 +--
target/i386/hax-darwin.c | 26 +++---
target/i386/hax-mem.c| 4 ++--
target/i386/hax-windows.c| 42 +--
target/i386/kvm.c| 38 +++
target/i386/misc_helper.c| 12 +-
target/lm32/op_helper.c | 4 ++--
target/mips/mips-semi.c | 3 ++-
target/mips/translate.c | 2 +-
target/ppc/excp_helper.c | 4 ++--
target/ppc/kvm.c | 36 +++---
target/ppc/mmu-hash64.c | 2 +-
target/ppc/mmu_helper.c | 2 +-
target/ppc/translate.c | 20 -
target/ppc/translate_init.c | 53 ++--
target/s390x/kvm.c | 20 -
target/s390x/misc_helper.c | 2 +-
target/sh4/translate.c | 4 ++--
target/unicore32/translate.c | 4 ++--
29 files changed, 188 insertions(+), 184 deletions(-)
diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index 25207cb850..2d56d5d579 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -9,6 +9,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "cpu.h"
#include "cpu-qom.h"
#include "internals.h"
@@ -24,7 +25,7 @@
#define DPRINTF(fmt, args...) \
do { \
if (DEBUG_ARM_POWERCTL) { \
-fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \
+error_report("[ARM]%s: " fmt , __func__, ##args); \
} \
} while (0)
@@ -32,7 +33,7 @@ CPUState *arm_get_cpu_by_id(uint64_t id)
{
CPUState *cpu;
-DPRINTF("cpu %" PRId64 "\n", id);
+DPRINTF("cpu %" PRId64 "", id);
CPU_FOREACH(cpu) {
ARMCPU *armcpu = ARM_CPU(cpu);
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 7cac8734c7..f8f12102f1 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -19,6 +19,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "cpu.h"
#include "exec/semihost.h"
@@ -649,7 +650,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
}
/* fall through -- invalid for A32/T32 */
default:
-fprintf(stderr, "qemu: Unsupported SemiHosting SWI 0x%02x\n", nr);
+error_report("qemu: Unsupported
