On Fri, Sep 04, 2026 at 10:15:21AM +0100, Peter Maydell wrote:
> On Fri, 4 Sept 2026 at 09:56, Markus Armbruster <[email protected]> wrote:
> >
> > Peter Xu <[email protected]> writes:
> >
> > > On Thu, Sep 03, 2026 at 02:46:37PM -0300, Fabiano Rosas wrote:
> > >> Peter Xu <[email protected]> writes:
> > >>
> > >> > On Wed, Sep 02, 2026 at 07:15:29PM -0300, Fabiano Rosas wrote:
> > >> >> Using newlines in the g_test_message is fine. It automatically adds
> > >> >> the '#' required by the TAP protocol to the start of each line.
> > >> >
> > >> > IIUC we have such check not because TAP, but because all these
> > >> > functions
> > >> > will append one newline at the end, hence it's not needed. IOW, if it
> > >> > applies to g_test_message(), I don't see why it doesn't apply to the
> > >> > rest.
> > >> > But maybe there're other reasons?
> > >> >
> > >> > To make it simpler, maybe we just call a few times g_test_message()?
> > >> >
> > >>
> > >> Not sure I understand your point, Peter. I want to be able to print nice
> > >> messages in patch 9:
> > >>
> > >> g_test_message("expected vs. found:\n\n%s\n---\n%s:%s", str, t2[match],
> > >> t2[match + 1]);
> > >>
> > >> # HMP output mismatch for entry at line 55:
> > >> # expected vs. found:
> > >> #
> > >> # max-bandwidth: 10356305952768 bytes/hour
> > >> # ---
> > >> # max-bandwidth: 10356305952768 bytes/second
> > >>
> > >> What would be the issue of having newlines here?
> > >
> > > No issue here that I can see. My question was, why you moved
> > > g_test_message() out only, but not all?
> > >
> > > My gut feeling is we check this because people forget that all these
> > > functions includes a newline.
> > >
> > > So if your point stands here that "newlines can be in the middle", they
> > > should apply to all, not one.
> >
> > I'm not sure I understand you correctly. If you suggest to permit
> > newlines in the middle of error_setg(), error_report() & friends, I
> > disagree. qapi/error.h:
> >
> > * The resulting message should be a single phrase, with no newline or
> > * trailing punctuation.
> >
> > If you want to provide additional information, use error_append_hint() /
> > error_printf().
>
> Right. These functions have a genuinely different set of semantics
> from g_test_message(), which is why Fabiano's patch only changes
> how checkpatch handles that g_test_message(), not the various
> QEMU error/warning functions.
But in this case g_test_message() implies one message to be emitted, shall
we follow the same rule that we should stick with g_test_message() without
newlines, and use it multiple times? I don't think I have a solid clue of
such, that is why I actually suggested dropping this patch and just invoke
the g_test_message() a few times.
I'm personally fine either way, if we go with this patch, I suggest:
- When repost/queue, add some real reasoning on why g_test_message() is
different from the others. I don't think it's relevant to TAP format..
- Please if any of you agree with Fabiano's change, provide one ACK..
Thanks for chimming in.
--
Peter Xu
