Hi
On Fri, Jul 8, 2022 at 5:56 PM Markus Armbruster <arm...@redhat.com> wrote:
> Marc-André Lureau <marcandre.lur...@gmail.com> writes:
>
> > Hi
> >
> > On Thu, Jul 7, 2022 at 4:25 PM Markus Armbruster <arm...@redhat.com>
> wrote:
> >
> >> marcandre.lur...@redhat.com writes:
> >>
> >> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >> >
> >> > Not needed outside monitor.c. Remove the needless stub.
> >> >
> >> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> >> > ---
> >> > include/monitor/monitor.h | 1 -
> >> > monitor/monitor.c | 3 ++-
> >> > stubs/error-printf.c | 5 -----
> >> > 3 files changed, 2 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> >> > index a4b40e8391db..44653e195b45 100644
> >> > --- a/include/monitor/monitor.h
> >> > +++ b/include/monitor/monitor.h
> >> > @@ -56,7 +56,6 @@ void monitor_register_hmp(const char *name, bool
> info,
> >> > void monitor_register_hmp_info_hrt(const char *name,
> >> > HumanReadableText
> *(*handler)(Error **errp));
> >> >
> >> > -int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> G_GNUC_PRINTF(1, 0);
> >> > int error_printf_unless_qmp(const char *fmt, ...) G_GNUC_PRINTF(1,
> 2);
> >> >
> >> > #endif /* MONITOR_H */
> >> > diff --git a/monitor/monitor.c b/monitor/monitor.c
> >> > index 86949024f643..ba4c1716a48a 100644
> >> > --- a/monitor/monitor.c
> >> > +++ b/monitor/monitor.c
> >> > @@ -273,7 +273,8 @@ int error_vprintf(const char *fmt, va_list ap)
> >> > return vfprintf(stderr, fmt, ap);
> >> > }
> >> >
> >> > -int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> >> > +G_GNUC_PRINTF(1, 0)
> >> > +static int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> >> > {
> >> > Monitor *cur_mon = monitor_cur();
> >> >
> >> > diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> >> > index 0e326d801059..1afa0f62ca26 100644
> >> > --- a/stubs/error-printf.c
> >> > +++ b/stubs/error-printf.c
> >> > @@ -16,8 +16,3 @@ int error_vprintf(const char *fmt, va_list ap)
> >> > }
> >> > return vfprintf(stderr, fmt, ap);
> >> > }
> >> > -
> >> > -int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> >> > -{
> >> > - return error_vprintf(fmt, ap);
> >> > -}
> >>
> >> When I write a printf-like utility function, I habitually throw in a
> >> vprintf-like function.
> >>
> >> Any particular reason for hiding this one? To avoid misunderstandings:
> >> I'm fine with hiding it if it's causing you trouble.
> >
> > I don't think I had an issue with it, only that I wrote tests for the
> > error-report.h API, and didn't see the need to cover a function that
> isn't
> > used outside the unit.
>
> I'd keep it and not worry about missing tests; the tests of
> error_printf_unless_qmp() exercise it fine.
>
ok
>
> >> Except I think we'd better delete than hide then: inline into
> >> error_printf_unless_qmp(). Makes sense?
> >
> > It can't be easily inlined because of the surrounding va_start/va_end
>
> Easily enough, I think:
>
ah yes indeed! :)
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 86949024f6..201a672ac6 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -273,27 +273,22 @@ int error_vprintf(const char *fmt, va_list ap)
> return vfprintf(stderr, fmt, ap);
> }
>
> -int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> -{
> - Monitor *cur_mon = monitor_cur();
> -
> - if (!cur_mon) {
> - return vfprintf(stderr, fmt, ap);
> - }
> - if (!monitor_cur_is_qmp()) {
> - return monitor_vprintf(cur_mon, fmt, ap);
> - }
> - return -1;
> -}
> -
> int error_printf_unless_qmp(const char *fmt, ...)
> {
> + Monitor *cur_mon = monitor_cur();
> va_list ap;
> int ret;
>
> va_start(ap, fmt);
> - ret = error_vprintf_unless_qmp(fmt, ap);
> + if (!cur_mon) {
> + ret = vfprintf(stderr, fmt, ap);
> + } else if (!monitor_cur_is_qmp()) {
> + ret = monitor_vprintf(cur_mon, fmt, ap);
> + } else {
> + ret = -1;
> + }
> va_end(ap);
> +
> return ret;
> }
>
>
>
--
Marc-André Lureau
