Re: [Qemu-devel] [PATCH] qemu-error: remove dependency of stubs on monitor

[Halil Pasic]

On 10/24/2016 01:52 PM, Paolo Bonzini wrote:
> 
> On 24/10/2016 12:34, Dr. David Alan Gilbert wrote:
>> > * Paolo Bonzini (address@hidden) wrote:
>>> >> Leave the implementation of error_printf, error_printf_unless_qmp
>>> >> and error_vprintf to libqemustub.a and monitor.c, so that we can
>>> >> remove the monitor_printf and monitor_vprintf stubs.
>>> >>
>>> >> Signed-off-by: Paolo Bonzini <address@hidden>
>>> >> ---
>>> >>         This should help shutting up the vmstate unit tests.
>> > 
>> > Why does this make it any easier than my patch?
>> > You're still going to need to add something stub specific to turn
>> > the output on and off.
> It makes it possible to override the functions independent of the rest
> of util/qemu-error.c. You can implement the functions in the test, simply as
> 
>     had_stderr_output = true;
> 
> and then assert that had_stderr_output is false or true depending on the
> test.
> 
> (It's also a useful starting point to fix the cur_mon race).
> 
> Consider this an RFC.  error_printf probably should stay in qemu-error.c
> since it can always call error_vprintf.
> 
> Paolo
> 

Agree with Paolo that the ability to provide a custom (test specific)
implementation is generally beneficial testing and IMHO superior
compared to the solution previously proposed by Dave. Unfortunately I do
not see how is this accomplished by the patch.

I still consider using Error objects in the code under test and
not reporting errors directly the cleanest approach.

Halil

signature.asc
Description: OpenPGP digital signature