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

[Markus Armbruster <armbru@redhat.com>]

Andrew Jones <drjones@redhat.com> writes:

> On Mon, Oct 26, 2015 at 03:13:43PM -0200, Eduardo Habkost wrote:
>> This replaces most fprintf(stderr) calls on vl.c with error_report().
>> 
>> 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.
>> 
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>> Not sure if this is appropriate post soft-freeze, but if we are going to apply
>> the max-cpus patch from Drew before 2.5.0, we could simply change all the
>> fprintf() calls in a single step.
>
> In addition to Markus' and Eric's comments, I think we should
>
> 1. make sure the first word's case is correct. Lowercase for phrases,
>    uppercase for sentences and proper nouns.
> 2. make sure the 'warning' prefix is consistent in all uses. This should
>    be a QEMU-wide change. Maybe we need an error_report_warn variant?
> 3. make sure past tense is used in phrases like "failed to do..."
> 4. only break the line if necessary. Some of the lines are broken even
>    though they could fit in 80 char. Probably the opposite exists too,
>    i.e. some are > 80.
>   
> I've tried to point out a few of the cases below that inspired me to
> to write these suggestions.

These rules all make sense to me.

Cleaning up existing error messages to conform to them is a lot of grunt
work.  Patches welcome.

In theory, making new error messages conform is a matter of patch
review.  In practice, I'm afraid it'll take written guidelines and at
least a local scarcity of bad examples to make patch review work even
moderately well there.

We discussed written guidelines in the "Coding style for errors" thread.
We need someone to synthesize it into a patch.