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

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(-)
> 

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

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

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