| Message ID | 20200416094141.65120-3-wipawel@amazon.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Small fixes and improvements | expand |
On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote: > The explicit LFCR sequence guarantees proper line by line formatting > in the output. > The '\n' character alone on some terminals is not automatically > converted to LFCR. > > Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de> Up until now, all console destinations have expected POSIX text semantics. I presume this is due to the COM1 use presented in the next patch? Unfortunately, this comes with collateral damage. # ./xtf-runner hvm64 example Executing 'xl create -p tests/example/test-hvm64-example.cfg' Executing 'xl console test-hvm64-example' Executing 'xl unpause test-hvm64-example' --- Xen Test Framework --- Found Xen: 4.14 Environment: HVM 64bit (Long mode 4 levels) Hello World Test result: SUCCESS Combined test results: test-hvm64-example CRASH which I believe is due to xenconsoled (or the intervening pty) also expanding \n to \r\n (and "Test result:" no longer being on the final line from xtf-runner's point of view). Xen also expands \n to \r\n for the console, so ends up emitting \r\r\n. Would it be better to have the com1 console handler do the expansion locally, to avoid interfering with the semantics of every other destination? That said... > --- > common/libc/vsnprintf.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c > index a49fd30..3202137 100644 > --- a/common/libc/vsnprintf.c > +++ b/common/libc/vsnprintf.c > @@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) > if ( *fmt != '%' ) > { > PUT(*fmt); > + > + /* > + * The '\n' character alone on some terminals is not automatically > + * converted to LFCR. > + * The explicit LFCR sequence guarantees proper line by line > + * formatting in the output. > + */ > + if ( *fmt == '\n' && str < end ) > + PUT('\r'); ... doesn't this end up putting out \n\r ? ~Andrew > + > continue; > } >
> On 16. Apr 2020, at 12:18, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > > > > On 16/04/2020 10:41, Pawel Wieczorkiewicz wrote: >> The explicit LFCR sequence guarantees proper line by line formatting >> in the output. >> The '\n' character alone on some terminals is not automatically >> converted to LFCR. >> >> Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de> > > Up until now, all console destinations have expected POSIX text semantics. > > I presume this is due to the COM1 use presented in the next patch? > No, this is not about that. > Unfortunately, this comes with collateral damage. > > # ./xtf-runner hvm64 example > Executing 'xl create -p tests/example/test-hvm64-example.cfg' > Executing 'xl console test-hvm64-example' > Executing 'xl unpause test-hvm64-example' > --- Xen Test Framework --- > > Found Xen: 4.14 > > Environment: HVM 64bit (Long mode 4 levels) > > Hello World > > Test result: SUCCESS > > > Combined test results: > test-hvm64-example CRASH > I never use xtf-runner script to execute tests. I do it the old fashion way: # xl create -c test-hvm64-example.cfg Parsing config from test-hvm64-example.cfg Guest cpuid information Native cpuid: 00000000:ffffffff -> 0000000d:756e6547:6c65746e:49656e69 00000001:ffffffff -> 000306e4:00400800:f7ba2203:1fcbfbff 00000002:ffffffff -> 76036301:00f0b2ff:00000000:00ca0000 00000003:ffffffff -> 00000000:00000000:00000000:00000000 00000004:00000000 -> 7c000121:01c0003f:0000003f:00000000 00000004:00000001 -> 7c000122:01c0003f:0000003f:00000000 00000004:00000002 -> 7c000143:01c0003f:000001ff:00000000 00000004:00000003 -> 7c000163:04c0003f:00004fff:00000006 00000004:00000004 -> 00000000:00000000:00000000:00000000 00000005:ffffffff -> 00000040:00000040:00000003:00001120 00000006:ffffffff -> 00000077:00000002:00000009:00000000 00000007:00000000 -> 00000000:00000281:00000000:9c000400 00000008:ffffffff -> 00000000:00000000:00000000:00000000 00000009:ffffffff -> 00000000:00000000:00000000:00000000 0000000a:ffffffff -> 07300403:00000000:00000000:00000603 0000000b:ffffffff -> 00000000:00000000:00000000:00000000 0000000c:ffffffff -> 00000000:00000000:00000000:00000000 0000000d:00000000 -> 00000007:00000240:00000340:00000000 0000000d:00000001 -> 00000001:00000000:00000000:00000000 0000000d:00000002 -> 00000100:00000240:00000000:00000000 40000000:ffffffff -> 40000005:566e6558:65584d4d:4d4d566e 40000001:ffffffff -> 0004000b:00000000:00000000:00000000 40000002:ffffffff -> 00000001:40000000:00000000:00000000 40000003:00000000 -> 00000006:00000000:002625a2:00000001 40000003:00000001 -> 57b3c4d2:00030755:ccccc210:ffffffff 40000003:00000002 -> 002625a2:00000000:00000000:00000000 40000004:00000000 -> 0000001c:00000000:00000ac9:00000000 40000005:ffffffff -> 00000000:00000000:00000000:00000000 40000100:ffffffff -> 00000000:00000000:00000000:00000000 80000000:ffffffff -> 80000008:00000000:00000000:00000000 80000001:ffffffff -> 00000000:00000000:00000001:2c100800 80000002:ffffffff -> 20202020:6e492020:286c6574:58202952 80000003:ffffffff -> 286e6f65:43202952:45205550:36322d35 80000004:ffffffff -> 76203037:20402032:30352e32:007a4847 80000005:ffffffff -> 00000000:00000000:00000000:00000000 80000006:ffffffff -> 00000000:00000000:01006040:00000000 80000007:ffffffff -> 00000000:00000000:00000000:00000000 80000008:ffffffff -> 0000302e:00001000:00000000:00000000 Test result: SUCCESS There is no \r added to the console. I am not using serial console for this example. Also, qemu seems to do the right thing and appends \r when it is not there. > which I believe is due to xenconsoled (or the intervening pty) also > expanding \n to \r\n (and "Test result:" no longer being on the final > line from xtf-runner's point of view). Xen also expands \n to \r\n for > the console, so ends up emitting \r\r\n. > > Would it be better to have the com1 console handler do the expansion > locally, to avoid interfering with the semantics of every other > destination? That said... com1 handler works fine even without this patch > >> --- >> common/libc/vsnprintf.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c >> index a49fd30..3202137 100644 >> --- a/common/libc/vsnprintf.c >> +++ b/common/libc/vsnprintf.c >> @@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) >> if ( *fmt != '%' ) >> { >> PUT(*fmt); >> + >> + /* >> + * The '\n' character alone on some terminals is not automatically >> + * converted to LFCR. >> + * The explicit LFCR sequence guarantees proper line by line >> + * formatting in the output. >> + */ >> + if ( *fmt == '\n' && str < end ) >> + PUT('\r'); > > ... doesn't this end up putting out \n\r ? yes, it does > > ~Andrew > >> + >> continue; >> } >> > Best Regards, Pawel Wieczorkiewicz wipawel@amazon.com Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On 16/04/2020 12:36, Wieczorkiewicz, Pawel wrote: >> Unfortunately, this comes with collateral damage. >> >> # ./xtf-runner hvm64 example >> Executing 'xl create -p tests/example/test-hvm64-example.cfg' >> Executing 'xl console test-hvm64-example' >> Executing 'xl unpause test-hvm64-example' >> --- Xen Test Framework --- >> >> Found Xen: 4.14 >> >> Environment: HVM 64bit (Long mode 4 levels) >> >> Hello World >> >> Test result: SUCCESS >> >> >> Combined test results: >> test-hvm64-example CRASH >> > I never use xtf-runner script to execute tests. I do it the old fashion way: > > # xl create -c test-hvm64-example.cfg > Parsing config from test-hvm64-example.cfg I presume you mean hvm64-cpuid here, but... > Guest cpuid information > Native cpuid: > 00000000:ffffffff -> 0000000d:756e6547:6c65746e:49656e69 > 00000001:ffffffff -> 000306e4:00400800:f7ba2203:1fcbfbff > 00000002:ffffffff -> 76036301:00f0b2ff:00000000:00ca0000 > 00000003:ffffffff -> 00000000:00000000:00000000:00000000 > 00000004:00000000 -> 7c000121:01c0003f:0000003f:00000000 > 00000004:00000001 -> 7c000122:01c0003f:0000003f:00000000 > 00000004:00000002 -> 7c000143:01c0003f:000001ff:00000000 > 00000004:00000003 -> 7c000163:04c0003f:00004fff:00000006 > 00000004:00000004 -> 00000000:00000000:00000000:00000000 > 00000005:ffffffff -> 00000040:00000040:00000003:00001120 > 00000006:ffffffff -> 00000077:00000002:00000009:00000000 > 00000007:00000000 -> 00000000:00000281:00000000:9c000400 > 00000008:ffffffff -> 00000000:00000000:00000000:00000000 > 00000009:ffffffff -> 00000000:00000000:00000000:00000000 > 0000000a:ffffffff -> 07300403:00000000:00000000:00000603 > 0000000b:ffffffff -> 00000000:00000000:00000000:00000000 > 0000000c:ffffffff -> 00000000:00000000:00000000:00000000 > 0000000d:00000000 -> 00000007:00000240:00000340:00000000 > 0000000d:00000001 -> 00000001:00000000:00000000:00000000 > 0000000d:00000002 -> 00000100:00000240:00000000:00000000 > 40000000:ffffffff -> 40000005:566e6558:65584d4d:4d4d566e > 40000001:ffffffff -> 0004000b:00000000:00000000:00000000 > 40000002:ffffffff -> 00000001:40000000:00000000:00000000 > 40000003:00000000 -> 00000006:00000000:002625a2:00000001 > 40000003:00000001 -> 57b3c4d2:00030755:ccccc210:ffffffff > 40000003:00000002 -> 002625a2:00000000:00000000:00000000 > 40000004:00000000 -> 0000001c:00000000:00000ac9:00000000 > 40000005:ffffffff -> 00000000:00000000:00000000:00000000 > 40000100:ffffffff -> 00000000:00000000:00000000:00000000 > 80000000:ffffffff -> 80000008:00000000:00000000:00000000 > 80000001:ffffffff -> 00000000:00000000:00000001:2c100800 > 80000002:ffffffff -> 20202020:6e492020:286c6574:58202952 > 80000003:ffffffff -> 286e6f65:43202952:45205550:36322d35 > 80000004:ffffffff -> 76203037:20402032:30352e32:007a4847 > 80000005:ffffffff -> 00000000:00000000:00000000:00000000 > 80000006:ffffffff -> 00000000:00000000:01006040:00000000 > 80000007:ffffffff -> 00000000:00000000:00000000:00000000 > 80000008:ffffffff -> 0000302e:00001000:00000000:00000000 > Test result: SUCCESS ... I have reproduced this locally. However, I'd argue that this it is a bug in xenconsoled rather than XTF. In particular, modifying XTF would result in xenconsoled writing out the logfile with windows line endings, which surely isn't intended. >>> --- >>> common/libc/vsnprintf.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c >>> index a49fd30..3202137 100644 >>> --- a/common/libc/vsnprintf.c >>> +++ b/common/libc/vsnprintf.c >>> @@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) >>> if ( *fmt != '%' ) >>> { >>> PUT(*fmt); >>> + >>> + /* >>> + * The '\n' character alone on some terminals is not automatically >>> + * converted to LFCR. >>> + * The explicit LFCR sequence guarantees proper line by line >>> + * formatting in the output. >>> + */ >>> + if ( *fmt == '\n' && str < end ) >>> + PUT('\r'); >> ... doesn't this end up putting out \n\r ? > yes, it does So the one type of line ending which isn't in common use? ~Andrew
On 20/04/2020 20:26, Andrew Cooper wrote: >>>> --- >>>> common/libc/vsnprintf.c | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c >>>> index a49fd30..3202137 100644 >>>> --- a/common/libc/vsnprintf.c >>>> +++ b/common/libc/vsnprintf.c >>>> @@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) >>>> if ( *fmt != '%' ) >>>> { >>>> PUT(*fmt); >>>> + >>>> + /* >>>> + * The '\n' character alone on some terminals is not automatically >>>> + * converted to LFCR. >>>> + * The explicit LFCR sequence guarantees proper line by line >>>> + * formatting in the output. >>>> + */ >>>> + if ( *fmt == '\n' && str < end ) >>>> + PUT('\r'); >>> ... doesn't this end up putting out \n\r ? >> yes, it does > So the one type of line ending which isn't in common use? Switching this to do \r\n does seem to fix the raw `xl create` problem you were seeing before, doesn't cause the double newlines as far as `./xtf-runner` is concerned, and doesn't appear to cause xenconsoled to write windows file endings. I'm still a little hesitant to do this unilaterally. Do we know what Linux usually emits via the console, because that will get us closer to whatever people actually test. ~Andrew
> On 20. Apr 2020, at 21:26, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. > > > > On 16/04/2020 12:36, Wieczorkiewicz, Pawel wrote: >>> Unfortunately, this comes with collateral damage. >>> >>> # ./xtf-runner hvm64 example >>> Executing 'xl create -p tests/example/test-hvm64-example.cfg' >>> Executing 'xl console test-hvm64-example' >>> Executing 'xl unpause test-hvm64-example' >>> --- Xen Test Framework --- >>> >>> Found Xen: 4.14 >>> >>> Environment: HVM 64bit (Long mode 4 levels) >>> >>> Hello World >>> >>> Test result: SUCCESS >>> >>> >>> Combined test results: >>> test-hvm64-example CRASH >>> >> I never use xtf-runner script to execute tests. I do it the old fashion way: >> >> # xl create -c test-hvm64-example.cfg >> Parsing config from test-hvm64-example.cfg > > I presume you mean hvm64-cpuid here, but... > >> Guest cpuid information >> Native cpuid: >> 00000000:ffffffff -> 0000000d:756e6547:6c65746e:49656e69 >> 00000001:ffffffff -> 000306e4:00400800:f7ba2203:1fcbfbff >> 00000002:ffffffff -> 76036301:00f0b2ff:00000000:00ca0000 >> 00000003:ffffffff -> 00000000:00000000:00000000:00000000 >> 00000004:00000000 -> 7c000121:01c0003f:0000003f:00000000 >> 00000004:00000001 -> 7c000122:01c0003f:0000003f:00000000 >> 00000004:00000002 -> 7c000143:01c0003f:000001ff:00000000 >> 00000004:00000003 -> 7c000163:04c0003f:00004fff:00000006 >> 00000004:00000004 -> 00000000:00000000:00000000:00000000 >> 00000005:ffffffff -> 00000040:00000040:00000003:00001120 >> 00000006:ffffffff -> 00000077:00000002:00000009:00000000 >> 00000007:00000000 -> 00000000:00000281:00000000:9c000400 >> 00000008:ffffffff -> 00000000:00000000:00000000:00000000 >> 00000009:ffffffff -> 00000000:00000000:00000000:00000000 >> 0000000a:ffffffff -> 07300403:00000000:00000000:00000603 >> 0000000b:ffffffff -> 00000000:00000000:00000000:00000000 >> 0000000c:ffffffff -> 00000000:00000000:00000000:00000000 >> 0000000d:00000000 -> 00000007:00000240:00000340:00000000 >> 0000000d:00000001 -> 00000001:00000000:00000000:00000000 >> 0000000d:00000002 -> 00000100:00000240:00000000:00000000 >> 40000000:ffffffff -> 40000005:566e6558:65584d4d:4d4d566e >> 40000001:ffffffff -> 0004000b:00000000:00000000:00000000 >> 40000002:ffffffff -> 00000001:40000000:00000000:00000000 >> 40000003:00000000 -> 00000006:00000000:002625a2:00000001 >> 40000003:00000001 -> 57b3c4d2:00030755:ccccc210:ffffffff >> 40000003:00000002 -> 002625a2:00000000:00000000:00000000 >> 40000004:00000000 -> 0000001c:00000000:00000ac9:00000000 >> 40000005:ffffffff -> 00000000:00000000:00000000:00000000 >> 40000100:ffffffff -> 00000000:00000000:00000000:00000000 >> 80000000:ffffffff -> 80000008:00000000:00000000:00000000 >> 80000001:ffffffff -> 00000000:00000000:00000001:2c100800 >> 80000002:ffffffff -> 20202020:6e492020:286c6574:58202952 >> 80000003:ffffffff -> 286e6f65:43202952:45205550:36322d35 >> 80000004:ffffffff -> 76203037:20402032:30352e32:007a4847 >> 80000005:ffffffff -> 00000000:00000000:00000000:00000000 >> 80000006:ffffffff -> 00000000:00000000:01006040:00000000 >> 80000007:ffffffff -> 00000000:00000000:00000000:00000000 >> 80000008:ffffffff -> 0000302e:00001000:00000000:00000000 >> Test result: SUCCESS > > ... I have reproduced this locally. > Cool! > However, I'd argue that this it is a bug in xenconsoled rather than > XTF. In particular, modifying XTF would result in xenconsoled writing > out the logfile with windows line endings, which surely isn't intended. > We can’t fix xenconsoled retrospectively, so I’d argue that we have to have a workaround in XTF (or somewhere else, I do not care much where). I plan to keep using XTF with various Xen versions. >>>> --- >>>> common/libc/vsnprintf.c | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c >>>> index a49fd30..3202137 100644 >>>> --- a/common/libc/vsnprintf.c >>>> +++ b/common/libc/vsnprintf.c >>>> @@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) >>>> if ( *fmt != '%' ) >>>> { >>>> PUT(*fmt); >>>> + >>>> + /* >>>> + * The '\n' character alone on some terminals is not automatically >>>> + * converted to LFCR. >>>> + * The explicit LFCR sequence guarantees proper line by line >>>> + * formatting in the output. >>>> + */ >>>> + if ( *fmt == '\n' && str < end ) >>>> + PUT('\r'); >>> ... doesn't this end up putting out \n\r ? >> yes, it does > > So the one type of line ending which isn't in common use? > As long as it works… additional benefit is simplicity. I did not want to mess with the stream and potentially cause more harm. > ~Andrew Best Regards, Pawel Wieczorkiewicz wipawel@amazon.com Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
diff --git a/common/libc/vsnprintf.c b/common/libc/vsnprintf.c index a49fd30..3202137 100644 --- a/common/libc/vsnprintf.c +++ b/common/libc/vsnprintf.c @@ -285,6 +285,16 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args) if ( *fmt != '%' ) { PUT(*fmt); + + /* + * The '\n' character alone on some terminals is not automatically + * converted to LFCR. + * The explicit LFCR sequence guarantees proper line by line + * formatting in the output. + */ + if ( *fmt == '\n' && str < end ) + PUT('\r'); + continue; }
The explicit LFCR sequence guarantees proper line by line formatting in the output. The '\n' character alone on some terminals is not automatically converted to LFCR. Signed-off-by: Pawel Wieczorkiewicz <wipawel@amazon.de> --- common/libc/vsnprintf.c | 10 ++++++++++ 1 file changed, 10 insertions(+)