[Qemu-devel] [PATCH 00/11] misc: Add trailing '\n' to qemu_log() calls

[Qemu-devel] [PATCH 00/11] misc: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Nothing very exciting here.
I sometimes miss to notice some trace events, running with -d unimp,trace...
then using 'grep ^...'. This is only due to a missing '\n' :)

Philippe Mathieu-Daudé (11):
  hw/sd/milkymist-memcard: Add trailing '\n' to qemu_log() call
  hw/digic: Add trailing '\n' to qemu_log() calls
  xilinx-dp: Add trailing '\n' to qemu_log() call
  ppc/pnv: Add trailing '\n' to qemu_log() calls
  hw/core/register: Add trailing '\n' to qemu_log() call
  hw/mips/boston: Add trailing '\n' to qemu_log() calls
  stellaris: Add trailing '\n' to qemu_log() calls
  target/arm: Add trailing '\n' to qemu_log() calls
  target/m68k: Add trailing '\n' to qemu_log() call
  RISC-V: Add trailing '\n' to qemu_log() calls
  RFC target/xtensa: Add trailing '\n' to qemu_log() calls

 hw/arm/stellaris.c        | 11 ++++++-----
 hw/char/digic-uart.c      |  4 ++--
 hw/core/register.c        |  2 +-
 hw/display/xlnx_dp.c      |  4 +++-
 hw/mips/boston.c          |  8 ++++----
 hw/ppc/pnv_core.c         |  4 ++--
 hw/sd/milkymist-memcard.c |  2 +-
 hw/timer/digic-timer.c    |  4 ++--
 target/arm/helper.c       |  4 ++--
 target/m68k/translate.c   |  2 +-
 target/riscv/op_helper.c  |  6 ++++--
 target/xtensa/translate.c |  6 +++---
 12 files changed, 31 insertions(+), 26 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 01/11] hw/sd/milkymist-memcard: Add trailing '\n' to qemu_log() call

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/milkymist-memcard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index fe1cccca76..fcbccf54ea 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -140,7 +140,7 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
             r = s->response[s->response_read_ptr++];
             if (s->response_read_ptr > s->response_len) {
                 qemu_log_mask(LOG_GUEST_ERROR, "milkymist_memcard: "
-                              "read more cmd bytes than available. Clipping.");
+                              "read more cmd bytes than available: clipping\n");
                 s->response_read_ptr = 0;
             }
         }
-- 
2.17.1

[Qemu-devel] [PATCH 02/11] hw/digic: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/char/digic-uart.c   | 4 ++--
 hw/timer/digic-timer.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/char/digic-uart.c b/hw/char/digic-uart.c
index 6ebcb87a40..ccc75eaa4d 100644
--- a/hw/char/digic-uart.c
+++ b/hw/char/digic-uart.c
@@ -60,7 +60,7 @@ static uint64_t digic_uart_read(void *opaque, hwaddr addr,
     default:
         qemu_log_mask(LOG_UNIMP,
                       "digic-uart: read access to unknown register 0x"
-                      TARGET_FMT_plx, addr << 2);
+                      TARGET_FMT_plx "\n", addr << 2);
     }
 
     return ret;
@@ -98,7 +98,7 @@ static void digic_uart_write(void *opaque, hwaddr addr, uint64_t value,
     default:
         qemu_log_mask(LOG_UNIMP,
                       "digic-uart: write access to unknown register 0x"
-                      TARGET_FMT_plx, addr << 2);
+                      TARGET_FMT_plx "\n", addr << 2);
     }
 }
 
diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
index e1fcf73c3e..4d73077207 100644
--- a/hw/timer/digic-timer.c
+++ b/hw/timer/digic-timer.c
@@ -73,7 +73,7 @@ static uint64_t digic_timer_read(void *opaque, hwaddr offset, unsigned size)
     default:
         qemu_log_mask(LOG_UNIMP,
                       "digic-timer: read access to unknown register 0x"
-                      TARGET_FMT_plx, offset);
+                      TARGET_FMT_plx "\n", offset);
     }
 
     return ret;
@@ -109,7 +109,7 @@ static void digic_timer_write(void *opaque, hwaddr offset,
     default:
         qemu_log_mask(LOG_UNIMP,
                       "digic-timer: read access to unknown register 0x"
-                      TARGET_FMT_plx, offset);
+                      TARGET_FMT_plx "\n", offset);
     }
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 03/11] xilinx-dp: Add trailing '\n' to qemu_log() call

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/display/xlnx_dp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 6715b9cc2b..c32ab083f8 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -1074,7 +1074,9 @@ static void xlnx_dp_avbufm_write(void *opaque, hwaddr offset, uint64_t value,
     case AV_BUF_STC_SNAPSHOT1:
     case AV_BUF_HCOUNT_VCOUNT_INT0:
     case AV_BUF_HCOUNT_VCOUNT_INT1:
-        qemu_log_mask(LOG_UNIMP, "avbufm: unimplmented");
+        qemu_log_mask(LOG_UNIMP, "avbufm: unimplemented register 0x%04"
+                                 PRIx64 "\n",
+                      offset << 2);
         break;
     default:
         s->avbufm_registers[offset] = value;
-- 
2.17.1

[Qemu-devel] [PATCH 04/11] ppc/pnv: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ppc/pnv_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index cbb64ad9e7..13ad7d9e04 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -97,7 +97,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
         val = 0x24f000000000000ull;
         break;
     default:
-        qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx,
+        qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx "\n",
                   addr);
     }
 
@@ -107,7 +107,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
 static void pnv_core_xscom_write(void *opaque, hwaddr addr, uint64_t val,
                                  unsigned int width)
 {
-    qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx,
+    qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx "\n",
                   addr);
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 05/11] hw/core/register: Add trailing '\n' to qemu_log() call

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/register.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/register.c b/hw/core/register.c
index 0741a1af32..d2d1636250 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -96,7 +96,7 @@ void register_write(RegisterInfo *reg, uint64_t val, uint64_t we,
     if (test) {
         qemu_log_mask(LOG_UNIMP,
                       "%s:%s writing %#" PRIx64 " to unimplemented bits:" \
-                      " %#" PRIx64 "",
+                      " %#" PRIx64 "\n",
                       prefix, reg->access->name, val, ac->unimp);
     }
 
-- 
2.17.1

[Qemu-devel] [PATCH 06/11] hw/mips/boston: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/boston.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index 5302e5c885..52cce19766 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -176,7 +176,7 @@ static uint64_t boston_platreg_read(void *opaque, hwaddr addr,
     uint32_t gic_freq, val;
 
     if (size != 4) {
-        qemu_log_mask(LOG_UNIMP, "%uB platform register read", size);
+        qemu_log_mask(LOG_UNIMP, "%uB platform register read\n", size);
         return 0;
     }
 
@@ -205,7 +205,7 @@ static uint64_t boston_platreg_read(void *opaque, hwaddr addr,
         val |= PLAT_DDR_CFG_MHZ;
         return val;
     default:
-        qemu_log_mask(LOG_UNIMP, "Read platform register 0x%" HWADDR_PRIx,
+        qemu_log_mask(LOG_UNIMP, "Read platform register 0x%" HWADDR_PRIx "\n",
                       addr & 0xffff);
         return 0;
     }
@@ -215,7 +215,7 @@ static void boston_platreg_write(void *opaque, hwaddr addr,
                                  uint64_t val, unsigned size)
 {
     if (size != 4) {
-        qemu_log_mask(LOG_UNIMP, "%uB platform register write", size);
+        qemu_log_mask(LOG_UNIMP, "%uB platform register write\n", size);
         return;
     }
 
@@ -237,7 +237,7 @@ static void boston_platreg_write(void *opaque, hwaddr addr,
         break;
     default:
         qemu_log_mask(LOG_UNIMP, "Write platform register 0x%" HWADDR_PRIx
-                      " = 0x%" PRIx64, addr & 0xffff, val);
+                      " = 0x%" PRIx64 "\n", addr & 0xffff, val);
         break;
     }
 }
-- 
2.17.1

[Qemu-devel] [PATCH 07/11] stellaris: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/stellaris.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index e886f54976..502a20842c 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -203,11 +203,11 @@ static uint64_t gptm_read(void *opaque, hwaddr offset,
             return s->rtc;
         }
         qemu_log_mask(LOG_UNIMP,
-                      "GPTM: read of TAR but timer read not supported");
+                      "GPTM: read of TAR but timer read not supported\n");
         return 0;
     case 0x4c: /* TBR */
         qemu_log_mask(LOG_UNIMP,
-                      "GPTM: read of TBR but timer read not supported");
+                      "GPTM: read of TBR but timer read not supported\n");
         return 0;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -836,11 +836,12 @@ static void stellaris_i2c_write(void *opaque, hwaddr offset,
         break;
     case 0x20: /* MCR */
         if (value & 1) {
-            qemu_log_mask(LOG_UNIMP, "stellaris_i2c: Loopback not implemented");
+            qemu_log_mask(LOG_UNIMP,
+                          "stellaris_i2c: Loopback not implemented\n");
         }
         if (value & 0x20) {
             qemu_log_mask(LOG_UNIMP,
-                          "stellaris_i2c: Slave mode not implemented");
+                          "stellaris_i2c: Slave mode not implemented\n");
         }
         s->mcr = value & 0x31;
         break;
@@ -1124,7 +1125,7 @@ static void stellaris_adc_write(void *opaque, hwaddr offset,
         s->sspri = value;
         break;
     case 0x28: /* PSSI */
-        qemu_log_mask(LOG_UNIMP, "ADC: sample initiate unimplemented");
+        qemu_log_mask(LOG_UNIMP, "ADC: sample initiate unimplemented\n");
         break;
     case 0x30: /* SAC */
         s->sac = value;
-- 
2.17.1

[Qemu-devel] [PATCH 08/11] target/arm: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/arm/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index f75aa6e9ca..1248d84e6f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4570,7 +4570,7 @@ void hw_breakpoint_update(ARMCPU *cpu, int n)
     case 4: /* unlinked address mismatch (reserved if AArch64) */
     case 5: /* linked address mismatch (reserved if AArch64) */
         qemu_log_mask(LOG_UNIMP,
-                      "arm: address mismatch breakpoint types not implemented");
+                      "arm: address mismatch breakpoint types not implemented\n");
         return;
     case 0: /* unlinked address match */
     case 1: /* linked address match */
@@ -4604,7 +4604,7 @@ void hw_breakpoint_update(ARMCPU *cpu, int n)
     case 8: /* unlinked VMID match (reserved if no EL2) */
     case 10: /* unlinked context ID and VMID match (reserved if no EL2) */
         qemu_log_mask(LOG_UNIMP,
-                      "arm: unlinked context breakpoint types not implemented");
+                      "arm: unlinked context breakpoint types not implemented\n");
         return;
     case 9: /* linked VMID match (reserved if no EL2) */
     case 11: /* linked context ID and VMID match (reserved if no EL2) */
-- 
2.17.1

[Qemu-devel] [PATCH 09/11] target/m68k: Add trailing '\n' to qemu_log() call

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/m68k/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 37d6ffd853..4b5dbdb51c 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1556,7 +1556,7 @@ DISAS_INSN(undef)
     /* ??? This is both instructions that are as yet unimplemented
        for the 680x0 series, as well as those that are implemented
        but actually illegal for CPU32 or pre-68020.  */
-    qemu_log_mask(LOG_UNIMP, "Illegal instruction: %04x @ %08x",
+    qemu_log_mask(LOG_UNIMP, "Illegal instruction: %04x @ %08x\n",
                   insn, s->insn_pc);
     gen_exception(s, s->insn_pc, EXCP_UNSUPPORTED);
 }
-- 
2.17.1

[Qemu-devel] [PATCH 10/11] RISC-V: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/riscv/op_helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 3abf52453c..aec7558e1b 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -293,7 +293,8 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         if ((val_to_write & 3) == 0) {
             env->stvec = val_to_write >> 2 << 2;
         } else {
-            qemu_log_mask(LOG_UNIMP, "CSR_STVEC: vectored traps not supported");
+            qemu_log_mask(LOG_UNIMP,
+                          "CSR_STVEC: vectored traps not supported\n");
         }
         break;
     case CSR_SCOUNTEREN:
@@ -320,7 +321,8 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         if ((val_to_write & 3) == 0) {
             env->mtvec = val_to_write >> 2 << 2;
         } else {
-            qemu_log_mask(LOG_UNIMP, "CSR_MTVEC: vectored traps not supported");
+            qemu_log_mask(LOG_UNIMP,
+                          "CSR_MTVEC: vectored traps not supported\n");
         }
         break;
     case CSR_MCOUNTEREN:
-- 
2.17.1

[Qemu-devel] [RFC PATCH 11/11] target/xtensa: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC because I think the qemu_log_mask() was previously intended to concatenate
output to stderr... (multiple registers described on a single line).

 target/xtensa/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 89db23852b..a11162eebe 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -2234,7 +2234,7 @@ static void translate_rur(DisasContext *dc, const uint32_t arg[],
         if (uregnames[par[0]].name) {
             tcg_gen_mov_i32(cpu_R[arg[0]], cpu_UR[par[0]]);
         } else {
-            qemu_log_mask(LOG_UNIMP, "RUR %d not implemented, ", par[0]);
+            qemu_log_mask(LOG_UNIMP, "RUR %d not implemented\n", par[0]);
         }
     }
 }
@@ -2375,7 +2375,7 @@ static void translate_slli(DisasContext *dc, const uint32_t arg[],
 {
     if (gen_window_check2(dc, arg[0], arg[1])) {
         if (arg[2] == 32) {
-            qemu_log_mask(LOG_GUEST_ERROR, "slli a%d, a%d, 32 is undefined",
+            qemu_log_mask(LOG_GUEST_ERROR, "slli a%d, a%d, 32 is undefined\n",
                           arg[0], arg[1]);
         }
         tcg_gen_shli_i32(cpu_R[arg[0]], cpu_R[arg[1]], arg[2] & 0x1f);
@@ -2571,7 +2571,7 @@ static void translate_wur(DisasContext *dc, const uint32_t arg[],
         if (uregnames[par[0]].name) {
             gen_wur(par[0], cpu_R[arg[0]]);
         } else {
-            qemu_log_mask(LOG_UNIMP, "WUR %d not implemented, ", par[0]);
+            qemu_log_mask(LOG_UNIMP, "WUR %d not implemented\n", par[0]);
         }
     }
 }
-- 
2.17.1

Re: [Qemu-devel] [PATCH 09/11] target/m68k: Add trailing '\n' to qemu_log() call

[Laurent Vivier]

Le 06/06/2018 à 17:21, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  target/m68k/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 37d6ffd853..4b5dbdb51c 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -1556,7 +1556,7 @@ DISAS_INSN(undef)
>      /* ??? This is both instructions that are as yet unimplemented
>         for the 680x0 series, as well as those that are implemented
>         but actually illegal for CPU32 or pre-68020.  */
> -    qemu_log_mask(LOG_UNIMP, "Illegal instruction: %04x @ %08x",
> +    qemu_log_mask(LOG_UNIMP, "Illegal instruction: %04x @ %08x\n",
>                    insn, s->insn_pc);
>      gen_exception(s, s->insn_pc, EXCP_UNSUPPORTED);
>  }
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH 03/11] xilinx-dp: Add trailing '\n' to qemu_log() call

[Alistair Francis]

On Wed, Jun 6, 2018 at 8:21 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/display/xlnx_dp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index 6715b9cc2b..c32ab083f8 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -1074,7 +1074,9 @@ static void xlnx_dp_avbufm_write(void *opaque, hwaddr offset, uint64_t value,
>      case AV_BUF_STC_SNAPSHOT1:
>      case AV_BUF_HCOUNT_VCOUNT_INT0:
>      case AV_BUF_HCOUNT_VCOUNT_INT1:
> -        qemu_log_mask(LOG_UNIMP, "avbufm: unimplmented");
> +        qemu_log_mask(LOG_UNIMP, "avbufm: unimplemented register 0x%04"
> +                                 PRIx64 "\n",
> +                      offset << 2);
>          break;
>      default:
>          s->avbufm_registers[offset] = value;
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH 05/11] hw/core/register: Add trailing '\n' to qemu_log() call

[Alistair Francis]

On Wed, Jun 6, 2018 at 8:21 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/register.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/core/register.c b/hw/core/register.c
> index 0741a1af32..d2d1636250 100644
> --- a/hw/core/register.c
> +++ b/hw/core/register.c
> @@ -96,7 +96,7 @@ void register_write(RegisterInfo *reg, uint64_t val, uint64_t we,
>      if (test) {
>          qemu_log_mask(LOG_UNIMP,
>                        "%s:%s writing %#" PRIx64 " to unimplemented bits:" \
> -                      " %#" PRIx64 "",
> +                      " %#" PRIx64 "\n",
>                        prefix, reg->access->name, val, ac->unimp);
>      }
>
> --
> 2.17.1
>
>

Re: [Qemu-devel] [RFC PATCH 11/11] target/xtensa: Add trailing '\n' to qemu_log() calls

[Max Filippov]

On Wed, Jun 6, 2018 at 8:21 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC because I think the qemu_log_mask() was previously intended to concatenate
> output to stderr... (multiple registers described on a single line).

No, there were no such intent.

>  target/xtensa/translate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Acked-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 00/11] misc: Add trailing '\n' to qemu_log() calls

[John Snow]

On 06/06/2018 11:21 AM, Philippe Mathieu-Daudé wrote:
> Nothing very exciting here.
> I sometimes miss to notice some trace events, running with -d unimp,trace...
> then using 'grep ^...'. This is only due to a missing '\n' :)
> 

so error_setg must be used WITHOUT \n and logging must happen with \n?

If we're sure that's the way we want to have things laid out, we really
ought to augment checkpatch to catch this -- because there's 0% chance
that we'll keep it straight on our own otherwise.

--js

> Philippe Mathieu-Daudé (11):
>   hw/sd/milkymist-memcard: Add trailing '\n' to qemu_log() call
>   hw/digic: Add trailing '\n' to qemu_log() calls
>   xilinx-dp: Add trailing '\n' to qemu_log() call
>   ppc/pnv: Add trailing '\n' to qemu_log() calls
>   hw/core/register: Add trailing '\n' to qemu_log() call
>   hw/mips/boston: Add trailing '\n' to qemu_log() calls
>   stellaris: Add trailing '\n' to qemu_log() calls
>   target/arm: Add trailing '\n' to qemu_log() calls
>   target/m68k: Add trailing '\n' to qemu_log() call
>   RISC-V: Add trailing '\n' to qemu_log() calls
>   RFC target/xtensa: Add trailing '\n' to qemu_log() calls
> 
>  hw/arm/stellaris.c        | 11 ++++++-----
>  hw/char/digic-uart.c      |  4 ++--
>  hw/core/register.c        |  2 +-
>  hw/display/xlnx_dp.c      |  4 +++-
>  hw/mips/boston.c          |  8 ++++----
>  hw/ppc/pnv_core.c         |  4 ++--
>  hw/sd/milkymist-memcard.c |  2 +-
>  hw/timer/digic-timer.c    |  4 ++--
>  target/arm/helper.c       |  4 ++--
>  target/m68k/translate.c   |  2 +-
>  target/riscv/op_helper.c  |  6 ++++--
>  target/xtensa/translate.c |  6 +++---
>  12 files changed, 31 insertions(+), 26 deletions(-)
> 

Re: [Qemu-devel] [PATCH 04/11] ppc/pnv: Add trailing '\n' to qemu_log() calls

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On Wed, Jun 06, 2018 at 12:21:21PM -0300, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I'm not really sure what series this was part of, but I've applied it
to ppc-for-3.0 anyway.

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/pnv_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index cbb64ad9e7..13ad7d9e04 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -97,7 +97,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
>          val = 0x24f000000000000ull;
>          break;
>      default:
> -        qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx,
> +        qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx "\n",
>                    addr);
>      }
>  
> @@ -107,7 +107,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
>  static void pnv_core_xscom_write(void *opaque, hwaddr addr, uint64_t val,
>                                   unsigned int width)
>  {
> -    qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx,
> +    qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx "\n",
>                    addr);
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 04/11] ppc/pnv: Add trailing '\n' to qemu_log() calls

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]

On 06/06/2018 11:16 PM, David Gibson wrote:
> On Wed, Jun 06, 2018 at 12:21:21PM -0300, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> I'm not really sure what series this was part of, but I've applied it
> to ppc-for-3.0 anyway.

I did an effort to split the series, then add all the recipients to the
cover but I missed your mail indeed, sorry!

here is the cover:
http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg01266.html

Thanks!

> 
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> 
>> ---
>>  hw/ppc/pnv_core.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
>> index cbb64ad9e7..13ad7d9e04 100644
>> --- a/hw/ppc/pnv_core.c
>> +++ b/hw/ppc/pnv_core.c
>> @@ -97,7 +97,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
>>          val = 0x24f000000000000ull;
>>          break;
>>      default:
>> -        qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx,
>> +        qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx "\n",
>>                    addr);
>>      }
>>  
>> @@ -107,7 +107,7 @@ static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
>>  static void pnv_core_xscom_write(void *opaque, hwaddr addr, uint64_t val,
>>                                   unsigned int width)
>>  {
>> -    qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx,
>> +    qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx "\n",
>>                    addr);
>>  }
>>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 00/11] misc: Add trailing '\n' to qemu_log() calls

[Peter Maydell]

On 6 June 2018 at 20:43, John Snow <jsnow@redhat.com> wrote:
> so error_setg must be used WITHOUT \n and logging must happen with \n?
>
> If we're sure that's the way we want to have things laid out, we really
> ought to augment checkpatch to catch this -- because there's 0% chance
> that we'll keep it straight on our own otherwise.

It's a historical side effect of them being basically separately
designed APIs. error_setg() takes the "give it the entire error
message" approach. logging is printf-style and gives the caller
flexibility to compose more complicated lines of logging with
multiple calls. Consistency between the two might be nice but
there are an awful lot of qemu_log calls in the codebase to check
and change to the other convention...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/11] misc: Add trailing '\n' to qemu_log() calls

[Peter Maydell]

On 6 June 2018 at 16:21, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Nothing very exciting here.
> I sometimes miss to notice some trace events, running with -d unimp,trace...
> then using 'grep ^...'. This is only due to a missing '\n' :)
>
> Philippe Mathieu-Daudé (11):
>   hw/sd/milkymist-memcard: Add trailing '\n' to qemu_log() call
>   hw/digic: Add trailing '\n' to qemu_log() calls
>   xilinx-dp: Add trailing '\n' to qemu_log() call
>   ppc/pnv: Add trailing '\n' to qemu_log() calls
>   hw/core/register: Add trailing '\n' to qemu_log() call
>   hw/mips/boston: Add trailing '\n' to qemu_log() calls
>   stellaris: Add trailing '\n' to qemu_log() calls
>   target/arm: Add trailing '\n' to qemu_log() calls
>   target/m68k: Add trailing '\n' to qemu_log() call
>   RISC-V: Add trailing '\n' to qemu_log() calls
>   RFC target/xtensa: Add trailing '\n' to qemu_log() calls
>
>  hw/arm/stellaris.c        | 11 ++++++-----
>  hw/char/digic-uart.c      |  4 ++--
>  hw/core/register.c        |  2 +-
>  hw/display/xlnx_dp.c      |  4 +++-
>  hw/mips/boston.c          |  8 ++++----
>  hw/ppc/pnv_core.c         |  4 ++--
>  hw/sd/milkymist-memcard.c |  2 +-
>  hw/timer/digic-timer.c    |  4 ++--
>  target/arm/helper.c       |  4 ++--
>  target/m68k/translate.c   |  2 +-
>  target/riscv/op_helper.c  |  6 ++++--
>  target/xtensa/translate.c |  6 +++---
>  12 files changed, 31 insertions(+), 26 deletions(-)


Applied to target-arm.next, since about half of these are arm
and the rest are trivial and have got ack/review by the relevant
maintainers.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/11] misc: Add trailing '\n' to qemu_log() calls

[John Snow]

On 06/08/2018 06:54 AM, Peter Maydell wrote:
> On 6 June 2018 at 20:43, John Snow <jsnow@redhat.com> wrote:
>> so error_setg must be used WITHOUT \n and logging must happen with \n?
>>
>> If we're sure that's the way we want to have things laid out, we really
>> ought to augment checkpatch to catch this -- because there's 0% chance
>> that we'll keep it straight on our own otherwise.
> 
> It's a historical side effect of them being basically separately
> designed APIs. error_setg() takes the "give it the entire error
> message" approach. logging is printf-style and gives the caller
> flexibility to compose more complicated lines of logging with
> multiple calls. Consistency between the two might be nice but
> there are an awful lot of qemu_log calls in the codebase to check
> and change to the other convention...
> 

Well, there's a pragmatic reason against having it be uniform.

If logging lets you build more complicated statements, perhaps a
checkpatch /warning/ that you've omitted the newline might be nice.

--js