Re: [Qemu-devel] [PATCH v2 01/16] vl.c: Replace fprintf(stderr) with error_report()

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> Replace most fprintf(stderr) calls on vl.c with error_report().
>
> Minimal changes were made in the error messages. Only the trailing
> newlines, "qemu:" and "error:" message prefixes were removed.
>
> The only remaining fprintf(stderr) calls are the ones at
> qemu_kill_report(), because the error mesage is split in multiple
> fprintf() calls.
>
> Reviewed-by: Markus Armbruster <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> Changes v1 -> v2:
> * Indentation fix at "No machine specified" error message
> ---
>  vl.c | 228 
> +++++++++++++++++++++++++++++++++----------------------------------
>  1 file changed, 112 insertions(+), 116 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index f5f7c3f..25b91fc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -674,9 +674,9 @@ void runstate_set(RunState new_state)
>      assert(new_state < RUN_STATE_MAX);
>  
>      if (!runstate_valid_transitions[current_run_state][new_state]) {
> -        fprintf(stderr, "ERROR: invalid runstate transition: '%s' -> '%s'\n",
> -                RunState_lookup[current_run_state],
> -                RunState_lookup[new_state]);
> +        error_report("invalid runstate transition: '%s' -> '%s'",
> +                     RunState_lookup[current_run_state],
> +                     RunState_lookup[new_state]);
>          abort();
>      }
>      trace_runstate_set(new_state);
> @@ -828,8 +828,8 @@ static void configure_rtc_date_offset(const char 
> *startdate, int legacy)
>          rtc_start_date = mktimegm(&tm);
>          if (rtc_start_date == -1) {
>          date_fail:
> -            fprintf(stderr, "Invalid date format. Valid formats are:\n"
> -                            "'2006-06-17T16:01:21' or '2006-06-17'\n");
> +            error_report("Invalid date format. Valid formats are:\n"
> +                         "'2006-06-17T16:01:21' or '2006-06-17'");

I'm afraid you violate error_report()'s contract:

/*
 * Print an error message to current monitor if we have one, else to stderr.
 * Format arguments like sprintf().  The result should not contain
 * newlines.
 * Prepend the current location and append a newline.
 * It's wrong to call this in a QMP monitor.  Use error_setg() there.
 */

Make it something like

            error_report("invalid date format");
            error_printf("valid formats: '2006-06-17T16:01:21'"
                        " or '2006-06-17'\n");

>              exit(1);
>          }
>          rtc_date_offset = qemu_time() - rtc_start_date;
[...]