On Sun, Mar 05, 2017 at 06:56:09PM -0300, Philippe Mathieu-Daudé wrote:
> Hi Krzysztof,
>
> On 03/05/2017 06:48 PM, Krzysztof Kozlowski wrote:
> > error_report() is preferred over fprintf() for logging errors. Also
> > remove square brackets [] and additional new line characters in printed
> > messages.
> >
> > Signed-off-by: Krzysztof Kozlowski <k...@kernel.org>
> > ---
> > hw/arm/exynos4_boards.c | 6 +++---
> > hw/timer/exynos4210_mct.c | 5 +++--
> > hw/timer/exynos4210_pwm.c | 11 +++++------
> > hw/timer/exynos4210_rtc.c | 16 +++++++---------
> > 4 files changed, 18 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> > index 0efa19405409..5cd94d402b52 100644
> > --- a/hw/arm/exynos4_boards.c
> > +++ b/hw/arm/exynos4_boards.c
> > @@ -22,6 +22,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/error-report.h"
> > #include "qemu-common.h"
> > #include "cpu.h"
> > #include "sysemu/sysemu.h"
> > @@ -101,9 +102,8 @@ static Exynos4210State
> > *exynos4_boards_init_common(MachineState *machine,
> > MachineClass *mc = MACHINE_GET_CLASS(machine);
> >
> > if (smp_cpus != EXYNOS4210_NCPUS && !qtest_enabled()) {
> > - fprintf(stderr, "%s board supports only %d CPU cores. Ignoring
> > smp_cpus"
> > - " value.\n",
> > - mc->name, EXYNOS4210_NCPUS);
> > + error_report("%s board supports only %d CPU cores. Ignoring
> > smp_cpus value.",
> > + mc->name, EXYNOS4210_NCPUS);
>
> ok, to inform the user
By inform you mean error_report or some lower level?
>
> > }
> >
> > exynos4_board_binfo.ram_size = exynos4_board_ram_size[board_type];
> > diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
> > index 6069116942a4..48041ab036a6 100644
> > --- a/hw/timer/exynos4210_mct.c
> > +++ b/hw/timer/exynos4210_mct.c
> > @@ -53,6 +53,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/error-report.h"
> > #include "hw/sysbus.h"
> > #include "qemu/timer.h"
> > #include "qemu/main-loop.h"
> > @@ -1364,8 +1365,8 @@ static void exynos4210_mct_write(void *opaque, hwaddr
> > offset,
> > case L0_TCNTO: case L1_TCNTO:
> > case L0_ICNTO: case L1_ICNTO:
> > case L0_FRCNTO: case L1_FRCNTO:
> > - fprintf(stderr, "\n[exynos4210.mct: write to RO register "
> > - TARGET_FMT_plx "]\n\n", offset);
> > + error_report("exynos4210.mct: write to RO register "
> > TARGET_FMT_plx,
> > + offset);
>
> no need to annoy the user here, it seems better to use
> qemu_log_mask(LOG_UNIMP, ...
>
> same for following diffs
Sure, I'll fix that as well.
Thanks for review.
Best regards,
Krzysztof
