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.


>
> 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


-- 
Marc-André Lureau

Reply via email to