On 2/28/20 6:32 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <phi...@redhat.com> writes:
>
>> The error_report API doesn't want trailing newline characters.
>> Remove it, to avoid and error when moving the code around:
>>
>> ERROR: Error messages should not contain newlines
>
> Commit 312fd5f2909 has a Coccinelle script. It should be committed and
> re-run.
>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> ---
>> block.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block.c b/block.c
>> index 1bdb9c679d..e466d15914 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -5994,7 +5994,7 @@ void bdrv_img_create(const char *filename, const char
>> *fmt,
> bs = bdrv_open(full_backing, NULL, backing_options, back_flags,
> &local_err);
> g_free(full_backing);
> if (!bs && size != -1) {
>> /* Couldn't open BS, but we have a size, so it's nonfatal */
>> warn_reportf_err(local_err,
>> "Could not verify backing image. "
>> - "This may become an error in future
>> versions.\n");
>> + "This may become an error in future versions.");
>> local_err = NULL;
>> } else if (!bs) {
>> /* Couldn't open bs, do not have size */
>
> warn_reportf_err() is a convenience function to error_prepend(),
> warn_report() and free @local_err.
OK now I see.
Why warn_reportf_err() doesn't take a 'Error **err' instead, to set err
to NULL after freeing *err?
>
> When @local_err holds a message like "pants on fire", the code before
> the patch prints something like
>
> qemu-system-x86_64: warning: Could not verify backing image. This may
> become an error in future versions.
> pants on fire
>
> The patch "improves" it to
>
> qemu-system-x86_64: warning: Could not verify backing image. This may
> become an error in future versions.pants on fire
>
> General advice: this misuse of warn_reportf_err() is an excusable
> mistake, but when you *test* the error path, you can't *not* see that
> the actual message is crap. Test your errors!
>
> Actual improvement:
>
> warn_reportf_err(local_err, "Could not verify backing image:
> ");
> error_printf("This may become an error in future versions.\n");
>
> This should print
>
> qemu-system-x86_64: warning: Could not verify backing image: pants on fire
> This may become an error in future versions.
>
