Re: [PATCH v5 14/24] monitor: move error_vprintf back to error-report.c

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Jan 13, 2026 at 02:38:19PM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > The current unit tests rely on monitor.o not being linked, such
>> > that the monitor stubs get linked instead. Since error_vprintf
>> > is in monitor.o this allows a stub error_vprintf impl to be used
>> > that calls g_test_message.
>> >
>> > This takes a different approach, with error_vprintf moving
>> > back to error-report.c such that it is always linked into the
>> > tests. The monitor_vprintf() stub is then changed to use
>> > g_test_message if QTEST_SILENT_ERRORS is set, otherwise it will
>> > return -1 and trigger error_vprintf to call vfprintf.
>> >
>> > The end result is functionally equivalent for the purposes of
>> > the unit tests.
>> >
>> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> > Reviewed-by: Eric Blake <eblake@redhat.com>
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

[...]

>> Without stubs, no change in behavior.
>> 
>> With both stubs, before the patch:
>> 
>>     monitor_vprintf() is not supposed to run, and aborts
>> 
>>     error_vprintf() calls g_test_message() for tests, else vfprintf()
>> 
>> afterwards:
>> 
>>     monitor_vprintf() calls g_test_message() and succeeds in tests, else
>>     fails
>> 
>>     error_vprintf() calls monitor_printf(), and when it fails falls back
>>     to vfprintf().
>> 
>> Alright, error_vprintf() behaves the same as before.
>> 
>> monitor_vprintf() no longer aborts.  Hmm.  What if we somehow acquire
>> calls?  In tests, they'll go to g_test_message(), which is fine, I
>> guess.  Outside tests, they'll fail.  So does the non-stub version
>> unless the current monitor is HMP.  Also fine, I guess.
>> 
>> Is it possible to link just one of the stubs?
>
> There is only 1 stub after this patch - for monitor_vprintf().
> error_vprintf() is never stubbed anymore.
>
> The interesting scenario outside there tests is the non-system emulator
> binaries.
>
> Those will not have the monitor code, so will get the monitor_vprintf
> stub. That will report g_test_initialized() == false, and so the stub
> will return -1.  error_vprintf() will see this -1 return value and
> thus call to vfprintf().
>
> So the behaviour is again the same as before this patch AFAICT for all
> linkage scenarios.

Thank you!

Reviewed-by: Markus Armbruster <armbru@redhat.com>