On 10/10/23 10:35, Nico Boehr wrote:
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?
Because it also toggles clearing the screen
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()
I'd rather have a loop than to nest it inside an if.
Also, please add protection against overflowing sccb->msg (max 4088 bytes
if I looked it up right).
I considered this but we already have a 2k length check before that and
I'd like to see someone print ~2k in a single call.
The question is if we want the complexity for something that we'll very
likely never hit.
+ 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?
