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

From: BALATON Zoltan <balaton@eik.bme.hu>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>,
	qemu-devel@nongnu.org, "Artyom Tarasenko" <atar4qemu@gmail.com>,
	"Chris Wulff" <crwulff@gmail.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	"Marek Vasut" <marex@denx.de>,
	"Max Filippov" <jcmvbkbc@gmail.com>,
	"Dr . David Alan Gilbert" <dave@treblig.org>,
	"Jiaxun Yang" <jiaxun.yang@flygoat.com>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>,
	"Yoshinori Sato" <ysato@users.sourceforge.jp>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-ppc@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>
Subject: Re: [PATCH-for-9.1 05/21] target/m68k: Replace qemu_printf() by monitor_printf() in monitor
Date: Wed, 24 Apr 2024 11:22:38 +0200 (CEST)	[thread overview]
Message-ID: <2a73ffa1-9b08-25f7-ae66-c3d748a8dd8a@eik.bme.hu> (raw)
In-Reply-To: <605a2b47-0ff3-e7b6-17c7-ececed274e5d@eik.bme.hu>

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

On Wed, 24 Apr 2024, BALATON Zoltan wrote:
> 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?

Meant hmp_puts.

> Or add a comment near monitor_puts to explain this for now.
> Regards,
> BALATON Zoltan