Luiz Capitulino <lcapitul...@redhat.com> writes:
> The monitor_vprintf() function now touches the 'mon' pointer
> before calling monitor_puts(), this causes block migration
> to segfault as its functions call monitor_printf() with a
> NULL 'mon'.
I figure this worked fine until commit 4a29a85d made monitor_vprintf()
dereference mon.
> To fix the problem this commit moves the 'mon' NULL check
> from monitor_puts() to monitor_vprintf().
>
> This can potentially hide bugs, but for some reason this has
> been the behavior for a long time.
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> ---
> monitor.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index b518cc4..ebd0282 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -177,9 +177,6 @@ static void monitor_puts(Monitor *mon, const char *str)
> {
> char c;
>
> - if (!mon)
> - return;
> -
> for(;;) {
> c = *str++;
> if (c == '\0')
> @@ -195,6 +192,9 @@ static void monitor_puts(Monitor *mon, const char *str)
>
> void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> {
> + if (!mon)
> + return;
> +
> if (mon->mc && !mon->mc->print_enabled) {
> qemu_error_new(QERR_UNDEFINED_ERROR);
> } else {
There are no other callers of monitor_puts(), so removing the check
there is okay.
Before the code motion, we throw QERR_UNDEFINED_ERROR on
monitor_vprintf(NULL, ...). Afterwards, we don't. Could you explain
why that's okay?
