Re: [PATCH v4 36/49] monitor: tighten monitor_printf*()

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Aug 25, 2026 at 11:09:43PM +0400, Marc-André Lureau wrote:
> Rename monitor_printf->monitor_hmp_printf, monitor_vprintf->
> monitor_hmp_vprintf, and monitor_printc->monitor_hmp_printc, changing
> the first parameter from Monitor * to MonitorHMP * to enforce type
> safety. The implementation is also simplified: monitor_hmp_vprintf now
> directly calls g_strdup_vprintf + monitor_puts, removing the virtual
> dispatch via moncls->vprintf.
> 
> The dev_print() callbacks are temporarily using the MONITOR_HMP(mon)
> cast, they are fixed in the following commits.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  audio/audio-hmp-cmds.c                  |   6 +-
>  backends/cryptodev-hmp-cmds.c           |   9 +-
>  block/monitor/block-hmp-cmds.c          | 170 +++++++++---------
>  chardev/char-hmp-cmds.c                 |  12 +-
>  disas/disas-mon.c                       |  10 +-
>  docs/devel/style.rst                    |   2 +-
>  docs/devel/writing-monitor-commands.rst |   8 +-
>  dump/dump-hmp-cmds.c                    |   5 +-
>  hw/char/virtio-serial-bus.c             |  10 +-
>  hw/core/machine-hmp-cmds.c              | 213 +++++++++++-----------
>  hw/core/sysbus.c                        |   5 +-
>  hw/hexagon/hexagon_tlb.c                |  44 ++---
>  hw/i386/kvm/xen-stubs.c                 |   6 +-
>  hw/i386/kvm/xen_evtchn.c                |  21 +--
>  hw/i386/sgx-hmp-stub.c                  |   3 +-
>  hw/i386/sgx.c                           |  29 ++-
>  hw/misc/auxbus.c                        |   9 +-
>  hw/misc/mos6522-stub.c                  |   3 +-
>  hw/net/rocker/rocker-hmp-cmds.c         | 146 ++++++++--------
>  hw/pci/pci-hmp-cmds.c                   | 114 ++++++------
>  hw/pci/pci-stub.c                       |   3 +-
>  hw/s390x/s390-skeys.c                   |   9 +-
>  hw/s390x/s390-stattrib.c                |  20 +--
>  hw/uefi/ovmf-log.c                      |   5 +-
>  hw/usb/bus.c                            |  11 +-
>  hw/usb/host-libusb.c                    |  21 ++-
>  hw/virtio/virtio-hmp-cmds.c             | 297 ++++++++++++++++---------------
>  hw/xen/xen-bus.c                        |   5 +-
>  include/disas/disas.h                   |   4 +-
>  include/monitor/hmp.h                   |  12 +-
>  migration/dirtyrate.c                   |  46 +++--
>  migration/migration-hmp-cmds.c          | 301 ++++++++++++++++----------------
>  monitor/hmp-cmds.c                      | 141 +++++++--------
>  monitor/hmp.c                           | 143 +++++++--------
>  monitor/monitor-internal.h              |   6 -
>  monitor/monitor.c                       |  33 ++--
>  net/net-hmp-cmds.c                      |  31 ++--
>  net/slirp.c                             |  31 ++--
>  qom/qom-hmp-cmds.c                      |  27 ++-
>  replay/replay-debugging.c               |   5 +-
>  stats/stats-hmp-cmds.c                  |  57 +++---
>  stubs/hmp-cmd-info_sev.c                |   3 +-
>  stubs/monitor-core.c                    |   2 +-
>  system/dirtylimit-hmp-cmds.c            |  10 +-
>  system/qdev-monitor.c                   |  19 +-
>  system/runstate-hmp-cmds.c              |  16 +-
>  system/tpm-hmp-cmds.c                   |  29 ++-
>  target/i386/cpu-apic.c                  |   3 +-
>  target/i386/monitor.c                   | 152 ++++++++--------
>  target/i386/sev.c                       |  35 ++--
>  target/m68k/monitor.c                   |   3 +-
>  target/ppc/monitor.c                    |   3 +-
>  target/riscv/monitor.c                  |  55 +++---
>  target/sh4/monitor.c                    |  29 ++-
>  target/sparc/monitor.c                  |   3 +-
>  target/xtensa/monitor.c                 |   3 +-
>  tests/unit/test-util-sockets.c          |   2 +-
>  tools/qemu-vnc/clipboard.c              |   4 +-
>  tools/qemu-vnc/stubs.c                  |   2 +-
>  trace/trace-hmp-cmds.c                  |  12 +-
>  ui/ui-hmp-cmds.c                        | 115 ++++++------
>  util/error-report.c                     |   2 +-
>  util/qemu-print.c                       |  11 +-
>  63 files changed, 1219 insertions(+), 1327 deletions(-)




> diff --git a/util/qemu-print.c b/util/qemu-print.c
> index 5d4143d425a1..5938f2b6c338 100644
> --- a/util/qemu-print.c
> +++ b/util/qemu-print.c
> @@ -13,6 +13,7 @@
>  #include "qemu/osdep.h"
>  #include "monitor/monitor.h"
>  #include "monitor/hmp.h"
> +#include "qom/object.h"
>  #include "qemu/qemu-print.h"
>  
>  /*
> @@ -23,8 +24,13 @@
>  int qemu_vprintf(const char *fmt, va_list ap)
>  {
>      Monitor *cur_mon = monitor_cur();
> +
> +    /* for all monitors: QMP & HMP */
>      if (cur_mon) {
> -        return monitor_vprintf(cur_mon, fmt, ap);
> +        /* don't use monitor_cur_hmp(), to avoid a second lookup */
> +        MonitorHMP *hmp = (MonitorHMP *)
> +            object_dynamic_cast(OBJECT(cur_mon), TYPE_MONITOR_HMP);
> +        return monitor_hmp_vprintf(hmp, fmt, ap);

This isn't the same semantics AFAICT.

Original code, if monitor_cur() == QMP, we call monitor_vprintf()
which will return -1.

New code, if monitor_cur() == QMP, we will get a NULL back from
object_dynamic_cast which we then pass into monitor_hmp_vprintf
which will then crash on monitor_puts() IIUC.

>      }
>      return vprintf(fmt, ap);
>  }
> @@ -55,7 +61,8 @@ int qemu_printf(const char *fmt, ...)
>  int qemu_vfprintf(FILE *stream, const char *fmt, va_list ap)
>  {
>      if (!stream) {
> -        return monitor_vprintf(monitor_cur(), fmt, ap);
> +        MonitorHMP *hmp = monitor_cur_hmp();
> +        return monitor_hmp_vprintf(hmp, fmt, ap);

Same, this should crash on QMP now IIUC.

>      }
>      return vfprintf(stream, fmt, ap);
>  }
> 
> -- 
> 2.55.0.543.g5ebe2ebe4ea8
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|