Daniel P. Berrangé <[email protected]> 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 <[email protected]>
> Reviewed-by: Eric Blake <[email protected]>
> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
> monitor/monitor.c | 15 ---------------
> stubs/error-printf.c | 18 ------------------
> stubs/meson.build | 1 -
> stubs/monitor-core.c | 14 +++++++++++++-
> util/error-report.c | 15 +++++++++++++++
> 5 files changed, 28 insertions(+), 35 deletions(-)
> delete mode 100644 stubs/error-printf.c
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 627a59b23e..6dc5a7016d 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -268,21 +268,6 @@ void monitor_printc(Monitor *mon, int c)
> monitor_printf(mon, "'");
> }
>
> -int error_vprintf(const char *fmt, va_list ap)
> -{
> - Monitor *cur_mon = monitor_cur();
> - /*
> - * This will return -1 if 'cur_mon' is NULL, or is QMP.
> - * IOW this will only print if in HMP, otherwise we
> - * fallback to stderr for QMP / no-monitor scenarios.
> - */
> - int ret = monitor_vprintf(cur_mon, fmt, ap);
> - if (ret == -1) {
> - ret = vfprintf(stderr, fmt, ap);
> - }
> - return ret;
> -}
> -
> static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
> /* Limit guest-triggerable events to 1 per second */
> [QAPI_EVENT_RTC_CHANGE] = { 1000 * SCALE_MS },
> diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> deleted file mode 100644
> index 1afa0f62ca..0000000000
> --- a/stubs/error-printf.c
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -#include "qemu/osdep.h"
> -#include "qemu/error-report.h"
> -#include "monitor/monitor.h"
> -
> -int error_vprintf(const char *fmt, va_list ap)
> -{
> - int ret;
> -
> - if (g_test_initialized() && !g_test_subprocess() &&
> - getenv("QTEST_SILENT_ERRORS")) {
> - char *msg = g_strdup_vprintf(fmt, ap);
> - g_test_message("%s", msg);
> - ret = strlen(msg);
> - g_free(msg);
> - return ret;
> - }
> - return vfprintf(stderr, fmt, ap);
> -}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 0b2778c568..3d77458a3f 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -3,7 +3,6 @@
> # below, so that it is clear who needs the stubbed functionality.
>
> stub_ss.add(files('cpu-get-clock.c'))
> -stub_ss.add(files('error-printf.c'))
> stub_ss.add(files('fdset.c'))
> stub_ss.add(files('iothread-lock.c'))
> stub_ss.add(files('is-daemonized.c'))
> diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
> index 1894cdfe1f..a7c32297c9 100644
> --- a/stubs/monitor-core.c
> +++ b/stubs/monitor-core.c
> @@ -18,5 +18,17 @@ void qapi_event_emit(QAPIEvent event, QDict *qdict)
>
> int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> {
> - abort();
> + /*
> + * Pretend 'g_test_message' is our monitor console to
> + * stop the caller sending messages to stderr
> + */
> + if (g_test_initialized() && !g_test_subprocess() &&
> + getenv("QTEST_SILENT_ERRORS")) {
> + char *msg = g_strdup_vprintf(fmt, ap);
> + g_test_message("%s", msg);
> + size_t ret = strlen(msg);
> + g_free(msg);
> + return ret;
> + }
> + return -1;
> }
> diff --git a/util/error-report.c b/util/error-report.c
> index 1b17c11de1..b262ad01cb 100644
> --- a/util/error-report.c
> +++ b/util/error-report.c
> @@ -29,6 +29,21 @@ bool message_with_timestamp;
> bool error_with_guestname;
> const char *error_guest_name;
>
> +int error_vprintf(const char *fmt, va_list ap)
> +{
> + Monitor *cur_mon = monitor_cur();
> + /*
> + * This will return -1 if 'cur_mon' is NULL, or is QMP.
> + * IOW this will only print if in HMP, otherwise we
> + * fallback to stderr for QMP / no-monitor scenarios.
> + */
> + int ret = monitor_vprintf(cur_mon, fmt, ap);
> + if (ret == -1) {
> + ret = vfprintf(stderr, fmt, ap);
> + }
> + return ret;
> +}
> +
> int error_printf(const char *fmt, ...)
> {
> va_list ap;
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?
