Quoting Janosch Frank (2023-10-10 09:38:54)
[...]
> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> index 19c74e46..313be1e4 100644
> --- a/lib/s390x/sclp-console.c
> +++ b/lib/s390x/sclp-console.c
[...]
> +static bool lpar_ascii_compat;
This only toggles adding \r. So why not name it accordingly?
Something like:
ascii_line_end_dos
or
ascii_add_cr_line_end
> static char lm_buff[120];
> static unsigned char lm_buff_off;
> static struct spinlock lm_buff_lock;
> @@ -97,14 +100,27 @@ static void sclp_print_ascii(const char *str)
> {
> int len = strlen(str);
> WriteEventData *sccb = (void *)_sccb;
> + char *str_dest = (char *)&sccb->msg;
> + int i = 0;
>
> sclp_mark_busy();
> memset(sccb, 0, sizeof(*sccb));
> +
> + for (; i < len; i++) {
> + *str_dest = str[i];
> + str_dest++;
> + /* Add a \r to the \n for HMC ASCII console */
> + if (str[i] == '\n' && lpar_ascii_compat) {
> + *str_dest = '\r';
> + str_dest++;
> + }
> + }
Please don't hide the check inside the loop.
Do:
if (lpar_ascii_compat)
// your loop
else
memcpy()
Also, please add protection against overflowing sccb->msg (max 4088 bytes
if I looked it up right).
> + len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;
And when you do the above, it should be easy to get rid of pointer
subtraction.
[...]
> void sclp_console_setup(void)
> {
> + lpar_ascii_compat = detect_host_early() == HOST_IS_LPAR;
> +
> /* We send ASCII and line mode. */
> sclp_write_event_mask(0, SCLP_EVENT_MASK_MSG_ASCII | SCLP_EVENT_MASK_MSG);
> + /* Hard terminal reset to clear screen for HMC ASCII console */
> + if (lpar_ascii_compat)
> + sclp_print_ascii("\ec");
I have in the past cursed programs which clear the screen, but I can see
the advantage here. How do others feel about this?
