Re: [PATCH v5 13/24] monitor: refactor error_vprintf()
On Wed, Jan 14, 2026 at 12:34:14PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé writes:
>
> > The monitor_vprintf() code will return -1 if either the monitor
> > is NULL, or the monitor is QMP. The error_vprintf() code can
> > take advantage of this to avoid having to duplicate the same
> > checks, and instead simply look at the return value.
> >
> > Reviewed-by: Richard Henderson
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > monitor/monitor.c | 16 +---
> > 1 file changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > index 4d26cd9496..627a59b23e 100644
> > --- a/monitor/monitor.c
> > +++ b/monitor/monitor.c
> > @@ -268,17 +268,19 @@ void monitor_printc(Monitor *mon, int c)
> > monitor_printf(mon, "'");
> > }
> >
> > -/*
> > - * Print to current monitor if we have one, else to stderr.
> > - */
>
> Keep the comment, please.
I'll change it to say "...to the current human monitor..."
>
> > int error_vprintf(const char *fmt, va_list ap)
> > {
> > Monitor *cur_mon = monitor_cur();
> > -
> > -if (cur_mon && !monitor_cur_is_qmp()) {
> > -return monitor_vprintf(cur_mon, fmt, ap);
> > +/*
> > + * 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 vfprintf(stderr, fmt, ap);
> > +return ret;
> > }
> >
> > static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v5 13/24] monitor: refactor error_vprintf()
Daniel P. Berrangé writes:
> The monitor_vprintf() code will return -1 if either the monitor
> is NULL, or the monitor is QMP. The error_vprintf() code can
> take advantage of this to avoid having to duplicate the same
> checks, and instead simply look at the return value.
>
> Reviewed-by: Richard Henderson
> Signed-off-by: Daniel P. Berrangé
> ---
> monitor/monitor.c | 16 +---
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 4d26cd9496..627a59b23e 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -268,17 +268,19 @@ void monitor_printc(Monitor *mon, int c)
> monitor_printf(mon, "'");
> }
>
> -/*
> - * Print to current monitor if we have one, else to stderr.
> - */
Keep the comment, please.
> int error_vprintf(const char *fmt, va_list ap)
> {
> Monitor *cur_mon = monitor_cur();
> -
> -if (cur_mon && !monitor_cur_is_qmp()) {
> -return monitor_vprintf(cur_mon, fmt, ap);
> +/*
> + * 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 vfprintf(stderr, fmt, ap);
> +return ret;
> }
>
> static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
Re: [PATCH v5 13/24] monitor: refactor error_vprintf()
Daniel P. Berrangé writes: > The monitor_vprintf() code will return -1 if either the monitor > is NULL, or the monitor is QMP. The error_vprintf() code can > take advantage of this to avoid having to duplicate the same > checks, and instead simply look at the return value. > > Reviewed-by: Richard Henderson > Signed-off-by: Daniel P. Berrangé Reviewed-by: Markus Armbruster
