* Markus Armbruster (arm...@redhat.com) wrote:
> Markus Armbruster writes:
>
> > printf() & friends return the number of characters written on success,
> > negative value on error.
> >
> > monitor_printf(), monitor_vfprintf(), monitor_vprintf(),
> > error_printf(), error_printf_unless_qmp(), error_vprintf(), and
> > error_vprintf_unless_qmp() return void. Some of them carry a TODO
> > comment asking for int instead.
> >
> > Improve them to return int like printf() does.
> >
> > This makes our use of monitor_printf() as fprintf_function slightly
> > less dirty: the function cast no longer adds a return value that isn't
> > there. It still changes a parameter's pointer type. That will be
> > addressed in a future commit.
> >
> > monitor_vfprintf() always returns zero. Improve it to return the
> > proper value.
> >
> > Cc: Dr. David Alan Gilbert
> > Signed-off-by: Markus Armbruster
> > ---
> [...]
> > diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> > index 99c6406668..537298209d 100644
> > --- a/stubs/error-printf.c
> > +++ b/stubs/error-printf.c
> > @@ -2,19 +2,19 @@
> > #include "qemu-common.h"
> > #include "qemu/error-report.h"
> >
> > -void error_vprintf(const char *fmt, va_list ap)
> > +int error_vprintf(const char *fmt, va_list ap)
> > {
> > if (g_test_initialized() && !g_test_subprocess() &&
> > getenv("QTEST_SILENT_ERRORS")) {
> > char *msg = g_strdup_vprintf(fmt, ap);
> > g_test_message("%s", msg);
> > g_free(msg);
> > -} else {
> > -vfprintf(stderr, fmt, ap);
> > +return strlen(msg);
>
> Use after free. Obvious fixup appended.
>
> > }
> > +return vfprintf(stderr, fmt, ap);
> > }
> >
> > -void error_vprintf_unless_qmp(const char *fmt, va_list ap)
> > +int error_vprintf_unless_qmp(const char *fmt, va_list ap)
> > {
> > -error_vprintf(fmt, ap);
> > +return error_vprintf(fmt, ap);
> > }
> [...]
>
> diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> index 537298209d..1f9d3b3714 100644
> --- a/stubs/error-printf.c
> +++ b/stubs/error-printf.c
> @@ -4,12 +4,15 @@
>
> int error_vprintf(const char *fmt, va_list ap)
> {
> +int ret;
> +
> if (g_test_initialized() && !g_test_subprocess() &&
> getenv("QTEST_SILENT_ERRORS")) {
> char *msg = g_strdup_vprintf(fmt, ap);
> g_test_message("%s", msg);
> +ret = strlen(msg);
> g_free(msg);
> -return strlen(msg);
> +return ret;
> }
> return vfprintf(stderr, fmt, ap);
> }
Yeh, with the fixup:
Reviewed-by: Dr. David Alan Gilbert
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
