Re: [Qemu-devel] [PATCH v1 8/8] target: Replace fprintf(stderr, "*\n" with error_report()

[Thomas Huth <thuth@redhat.com>]

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