Re: [PATCH] xen/efi: Reuse PrintMessage function

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/07/2025 1:00 pm, Frediano Ziglio wrote:
> On Fri, Jul 18, 2025 at 12:12 PM Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 18/07/2025 10:41 am, Frediano Ziglio wrote:
>>> PrintMessage function print a message string followed by a
>>> new line.
>>> Move the function from ARM specific code to common code.
>>> Reuse it in EFI code.
>>> No functional changes.
>>>
>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
>> Please no.
>>
>> Hiding \n (or \r\n) in strings is an antipattern and a source of
>> confusion that we do not want in Xen.
>>
> I didn't realize it was not a Xen pattern.

We went through a phase where it was inconsistent and we kept on having
problems on the console.  Missing a newline causes the next message to
be appended with <0> and it's very confusion to read.

>  I found the thing not that
> coherent between x86 and arm and attempted to fix it.

Sure, and making things consistent is very welcome.

>
>> Instead, delete PrintMessage() and convert ARM to use PrintStr().  That
>> looks like it was premature anti-optimisation.  You save 4 bytes per
>> string, but it costs 12 byte minimum in .text to set up the function
>> call, let alone the function call itself.
>>
> To be honest I didn't consider this aspect and I don't feel it matters much.
> Looking at other languages printing an implicit newline is pretty
> common, Pascal has "Wtite" and "WtiteLn", Ocaml has "print_string" and
> "print_endline",

Right, and in both cases these are clear from context (i.e. the function
name) that there's a newline.

PrintStr() and PrintMessage() do not have the same kind of clarity.

>  Python has an "end" parameter which by default is the
> newline, Basic (if I remember) has the possibility of an implicit
> empty field. In C is not so common although there's "puts" and some
> functions (like "syslog") have some kind of implicit newline.
> But obviously Xen is not any of them.
>
> As I said above however my main reason is coherency so I'll post your
> suggestion.

Thankyou.  I'm sure ARM will be better off as a result.

~Andrew