[v5] util: sync error_report & qemu_log output more closely

[PATCH v5 13/24] monitor: refactor error_vprintf()

Posted by Daniel P. Berrangé 3 weeks, 3 days ago

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 <richard.henderson@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 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.
- */
 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] = {
-- 
2.52.0

Re: [PATCH v5 13/24] monitor: refactor error_vprintf()

Posted by Markus Armbruster 2 weeks, 4 days ago

Daniel P. Berrangé <berrange@redhat.com> 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 <richard.henderson@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  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()

Posted by Daniel P. Berrangé 2 weeks, 4 days ago

On Wed, Jan 14, 2026 at 12:34:14PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> 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 <richard.henderson@linaro.org>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  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()

Posted by Markus Armbruster via Devel 2 weeks, 5 days ago

Daniel P. Berrangé <berrange@redhat.com> 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 <richard.henderson@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

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