Re: [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

On Wed, 24 Apr 2024, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> Replace qemu_printf() by monitor_printf() / monitor_puts() in monitor.
>
> Why?  Here's my attempt at an answer: because this runs only within HMP
> command "info tlb".  Using qemu_printf() there isn't wrong, but with
> monitor_printf(), it's obvious that we print to the monitor.
>
> On monitor_printf() vs. monitor_puts().
>
> qemu_printf() behaves like monitor_printf() when monitor_cur() returns
> non-null, which it certainly does within a monitor command.
>
> monitor_printf() prints like monitor_puts() when monitor_is_qmp()
> returns false, which it certainly does within an HMP command.
>
> Note: despite their names, monitor_printf() and monitor_puts() are at
> different interface layers!
>
> 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(), ...
>
> Naming the functions that expect an HMP monitor hmp_FOO() would make
> more sense.  Renaming them now would be quite some churn, though.
> Discussed at
> <https://lore.kernel.org/qemu-devel/87y1adm0os.fsf@pond.sub.org/>.

The hmp_ prefix is more cryptic than monitor_. Without knowing QEMU too 
much I can guess what monitor_ does but would have to look up what 
hmp_means so keeping monitor_ is better IMO. The solution to the naming 
issue mentioned above may be renaming monitor_puts to something that tells 
it's a low level function (and add a momitor_puts that behaves as 
expected) but I can't come up with a name either. Maybe the low level 
function could be called hmp_putt? Or add a comment near monitor_puts to 
explain this for now.

Regards,
BALATON Zoltan