Luiz Capitulino <lcapitul...@redhat.com> writes:
> On Tue, 14 Jan 2014 17:44:51 +0100
> Kevin Wolf <kw...@redhat.com> wrote:
>
>> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
>> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
>> > > Ping,
>> > >
>> > > Has this one been forgotten or are there issues? PMM had a small
>> > > comment, but he waived it AFAICT.
>> >
>> > Pong,
>> >
>> > I've merged it now, thanks!
>>
>> I believe it's something in this pull requests that breaks make check.
>
> And you're right. But first, let me confirm that we're talking about the
> same breakage. This is what I'm getting:
>
> make tests/check-qom-interface
> libqemuutil.a(qemu-error.o): In function `error_vprintf':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23:
> undefined reference to `cur_mon'
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24:
> undefined reference to `cur_mon'
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24:
> undefined reference to `monitor_vprintf'
> libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47:
> undefined reference to `monitor_cur_is_qmp'
> libqemuutil.a(qemu-error.o): In function `error_print_loc':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174:
> undefined reference to `cur_mon'
> collect2: error: ld returned 1 exit status
> make: *** [tests/check-qom-interface] Error 1
>
> I tried bisecting it, but git bisect weren't capable of finding the
> culprit. So debugged it, and the problem was introduced by:
>
> commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
> Author: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> Date: Wed Jan 1 18:49:52 2014 -0800
>
> qerror: Remove assert_no_error()
>
Are you sure you don't mean commit 5d24ee7 "error: Add error_abort"?
> There isn't nothing really wrong with this commit. The problem seems to
> be that the tests link against libqemuutil.a and this library pulls in
> everything from util/. The commit above changed util/error.c to call
> error_report(),
Yes, 5d24ee7 does that.
> which depends on 'cur_mon', which is only made available
> by monitor.o.
And stubs/mon-set-error.o
> I don't think we want to mess up with including monitor.o on libqemuutil.a.
> Besides, I now find it a bit weird to call error_report() from an error
> reporting function. So it's better to just call fprintf(stderr,) instead.
It's not weird at all. Higher layer calls lower layer.
> Peter, Markus, are you ok with this patch?
>
> PS: I do run make check before sending a pull request, and did run this
> time too. Not sure how I didn't catch this. Thanks for the report
> Kevin!
>
> diff --git a/util/error.c b/util/error.c
> index f11f1d5..7c7650c 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const
> char *fmt, ...)
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass
> err_class,
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err,
> ErrorClass err_class,
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -171,7 +171,7 @@ void error_free(Error *err)
> void error_propagate(Error **dst_err, Error *local_err)
> {
> if (local_err && dst_err == &error_abort) {
> - error_report("%s", error_get_pretty(local_err));
> + fprintf(stderr, "%s", error_get_pretty(local_err));
> abort();
> } else if (dst_err && !*dst_err) {
> *dst_err = local_err;
Error message screwed up!
Before:
$ qemu -msg timestamp=on -global i440FX-pcihost.foo=bar
2014-01-15T09:50:18.066725Z upstream-qemu: Property '.foo' not found
Aborted (core dumped)
After:
Property '.foo' not foundAborted (core dumped)
Note the loss of timestamp, name of executable, location, and the final
newline. Please fix.
Amazing super-secret trick to get error messages fit for human
consumption: reproduce them before you commit! ;-P
