Peter Maydell <peter.mayd...@linaro.org> writes: > On Wed, 20 Mar 2024 at 13:03, Daniel P. Berrangé <berra...@redhat.com> wrote: >> >> On Wed, Mar 20, 2024 at 04:36:40PM +0800, Tao Su wrote: >> > monitor_puts() doesn't check the monitor pointer, but do_inject_x86_mce() >> > may have a parameter with NULL monitor pointer. Revert monitor_puts() in >> > do_inject_x86_mce() to fix, then the fact that we send the same message to >> > monitor and log is again more obvious. >> >> Yikes, why do we have such a horrible trap-door in our >> monitor output APIs. >> >> Isn't the right fix here to make 'monitor_puts' check for >> NULL & be a no-op, in the same way 'monitor_printf' does, >> so the APIs have consistent behaviour. > > The other difference between monitor_puts(mon, s) and > monitor_printf(mon, "%s", s) > is that the latter will return an error if the monitor is QMP, whereas > the former will go ahead and print the message anyway. That one is > awkward to resolve, because the mechanism the QMP monitor uses to > print the JSON in qmp_send_response() is to call monitor_puts()...
We need a low-level function to send to a monitor, be it HMP or QMP: monitor_puts(). We need a high-level function to format JSON and send it to QMP: qmp_send_response(). We need a high-level functions to format text and send it to HMP: monitor_printf(), ... Trouble is the first and the last one are deceptively named. The names suggest monitor_printf() is to monitor_puts() what printf() is to puts(). Not true. Naming the functions that expect an HMP monitor hmp_FOO() would make more sense. Renaming them now would be quite some churn, though.