diff mbox series

[kvm-unit-tests,RFC,1/3] lib: s390x: sclp: Add carriage return to line feed

Message ID 20230630145449.2312-2-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Improve console handling | expand

Commit Message

Janosch Frank June 30, 2023, 2:54 p.m. UTC
Without the \r the output of the ASCII console takes a lot of
additional effort to read in comparison to the line mode console.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 lib/s390x/sclp-console.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Claudio Imbrenda June 30, 2023, 3:12 p.m. UTC | #1
On Fri, 30 Jun 2023 14:54:47 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Without the \r the output of the ASCII console takes a lot of
> additional effort to read in comparison to the line mode console.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  lib/s390x/sclp-console.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> index 19c74e46..384080b0 100644
> --- a/lib/s390x/sclp-console.c
> +++ b/lib/s390x/sclp-console.c
> @@ -97,14 +97,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 */
> +		if (str[i] == '\n') {
> +			*str_dest = '\r';
> +			str_dest++;
> +		}
> +	}
> +
> +	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;

some strings will therefore potentially overflow the SCCB

sclp_print() refuses to print more than 2kB, with this patch that limit
could potentially be crossed

can you please briefly explain in a comment why that is ok? (or maybe
that is not ok? then fix it somehow :) )

>  	sccb->h.length = offsetof(WriteEventData, msg) + len;
>  	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
>  	sccb->ebh.length = sizeof(EventBufferHeader) + len;
>  	sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
> -	memcpy(&sccb->msg, str, len);
>  
>  	sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
>  }
Janosch Frank July 3, 2023, 11:46 a.m. UTC | #2
On 6/30/23 17:12, Claudio Imbrenda wrote:
> On Fri, 30 Jun 2023 14:54:47 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> Without the \r the output of the ASCII console takes a lot of
>> additional effort to read in comparison to the line mode console.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   lib/s390x/sclp-console.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
>> index 19c74e46..384080b0 100644
>> --- a/lib/s390x/sclp-console.c
>> +++ b/lib/s390x/sclp-console.c
>> @@ -97,14 +97,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 */
>> +		if (str[i] == '\n') {
>> +			*str_dest = '\r';
>> +			str_dest++;
>> +		}
>> +	}
>> +
>> +	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;
> 
> some strings will therefore potentially overflow the SCCB
> 
> sclp_print() refuses to print more than 2kB, with this patch that limit
> could potentially be crossed
> 
> can you please briefly explain in a comment why that is ok? (or maybe
> that is not ok? then fix it somehow :) )

I'd like to see someone find a useful application for printing 2kb in a 
single printf() call.

Anyway, I could truncate the ASCII after the 2KB limit when adding the \r.

I'm wondering how the line-mode console interprets the \r. If it ignores 
it, then we could also convert to \n\r for both consoles and check for 
2kb when converting.
Claudio Imbrenda July 3, 2023, 12:04 p.m. UTC | #3
On Mon, 3 Jul 2023 13:46:29 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 6/30/23 17:12, Claudio Imbrenda wrote:
> > On Fri, 30 Jun 2023 14:54:47 +0000
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> Without the \r the output of the ASCII console takes a lot of
> >> additional effort to read in comparison to the line mode console.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> ---
> >>   lib/s390x/sclp-console.c | 15 ++++++++++++++-
> >>   1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
> >> index 19c74e46..384080b0 100644
> >> --- a/lib/s390x/sclp-console.c
> >> +++ b/lib/s390x/sclp-console.c
> >> @@ -97,14 +97,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 */
> >> +		if (str[i] == '\n') {
> >> +			*str_dest = '\r';
> >> +			str_dest++;
> >> +		}
> >> +	}
> >> +
> >> +	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;  
> > 
> > some strings will therefore potentially overflow the SCCB
> > 
> > sclp_print() refuses to print more than 2kB, with this patch that limit
> > could potentially be crossed
> > 
> > can you please briefly explain in a comment why that is ok? (or maybe
> > that is not ok? then fix it somehow :) )  
> 
> I'd like to see someone find a useful application for printing 2kb in a 
> single printf() call.

I'm with you there

> 
> Anyway, I could truncate the ASCII after the 2KB limit when adding the \r.

sounds like a plan

> 
> I'm wondering how the line-mode console interprets the \r. If it ignores 
> it, then we could also convert to \n\r for both consoles and check for 
> 2kb when converting.

is \n\r the right order? what about \r\n ?
diff mbox series

Patch

diff --git a/lib/s390x/sclp-console.c b/lib/s390x/sclp-console.c
index 19c74e46..384080b0 100644
--- a/lib/s390x/sclp-console.c
+++ b/lib/s390x/sclp-console.c
@@ -97,14 +97,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 */
+		if (str[i] == '\n') {
+			*str_dest = '\r';
+			str_dest++;
+		}
+	}
+
+	len = (uintptr_t)str_dest - (uintptr_t)&sccb->msg;
 	sccb->h.length = offsetof(WriteEventData, msg) + len;
 	sccb->h.function_code = SCLP_FC_NORMAL_WRITE;
 	sccb->ebh.length = sizeof(EventBufferHeader) + len;
 	sccb->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
-	memcpy(&sccb->msg, str, len);
 
 	sclp_service_call(SCLP_CMD_WRITE_EVENT_DATA, sccb);
 }