Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
On 08.06.2018 17:58, Patryk Olszewski wrote: > W dniu 08.06.2018 o 17:25, Peter Maydell pisze: >> On 8 June 2018 at 06:47, Thomas Huth wrote: >>> On 07.06.2018 23:08, Philippe Mathieu-Daudé wrote: Remove the 'stair-step output' on stdio. This partially reverts commit 12fb0ac05, which was correct on the mailing list but got corrupted by the maintainer :p Introduced-by: 3b876140-c035-dd39-75d0-d54c48128...@redhat.com Reported-by: BALATON Zoltan Suggested-by: Thomas Huth Tested-by: Laurent Desnogues Signed-off-by: Philippe Mathieu-Daudé --- See: http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06202.html (bug) http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg01309.html (report) Peter, Can this enters directly as bug-fix? chardev/char-stdio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c index d83e60e787..96375f2ab8 100644 --- a/chardev/char-stdio.c +++ b/chardev/char-stdio.c @@ -59,7 +59,7 @@ static void qemu_chr_set_echo_stdio(Chardev *chr, bool echo) if (!echo) { tty.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP | INLCR | IGNCR | ICRNL | IXON); -tty.c_oflag &= ~OPOST; +tty.c_oflag |= OPOST; tty.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN); tty.c_cflag &= ~(CSIZE | PARENB); tty.c_cflag |= CS8; >>> I think this is the right way to go. >>> >>> Reviewed-by: Thomas Huth >> Applied to master, thanks. >> >> -- PMM >> > I actually think it would be better to set c_oflag to (OPOST | ONLCR) to > avoid any problems in the future. At this point it is assumed that ONLCR > is set. stdio output worked fine without explicitly setting ONLCR in the past, so unless we hit a situation where it is really required, I'd rather keep it that way now to avoid yet another unexpected regression. Thomas
Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
W dniu 08.06.2018 o 17:25, Peter Maydell pisze: > On 8 June 2018 at 06:47, Thomas Huth wrote: >> On 07.06.2018 23:08, Philippe Mathieu-Daudé wrote: >>> Remove the 'stair-step output' on stdio. >>> >>> This partially reverts commit 12fb0ac05, which was correct >>> on the mailing list but got corrupted by the maintainer :p >>> >>> Introduced-by: 3b876140-c035-dd39-75d0-d54c48128...@redhat.com >>> Reported-by: BALATON Zoltan >>> Suggested-by: Thomas Huth >>> Tested-by: Laurent Desnogues >>> Signed-off-by: Philippe Mathieu-Daudé >>> --- >>> See: >>> http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06202.html (bug) >>> http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg01309.html >>> (report) >>> >>> Peter, Can this enters directly as bug-fix? >>> >>> chardev/char-stdio.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c >>> index d83e60e787..96375f2ab8 100644 >>> --- a/chardev/char-stdio.c >>> +++ b/chardev/char-stdio.c >>> @@ -59,7 +59,7 @@ static void qemu_chr_set_echo_stdio(Chardev *chr, bool >>> echo) >>> if (!echo) { >>> tty.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP >>> | INLCR | IGNCR | ICRNL | IXON); >>> -tty.c_oflag &= ~OPOST; >>> +tty.c_oflag |= OPOST; >>> tty.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN); >>> tty.c_cflag &= ~(CSIZE | PARENB); >>> tty.c_cflag |= CS8; >>> >> I think this is the right way to go. >> >> Reviewed-by: Thomas Huth > Applied to master, thanks. > > -- PMM > I actually think it would be better to set c_oflag to (OPOST | ONLCR) to avoid any problems in the future. At this point it is assumed that ONLCR is set.
Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
On 8 June 2018 at 06:47, Thomas Huth wrote: > On 07.06.2018 23:08, Philippe Mathieu-Daudé wrote: >> Remove the 'stair-step output' on stdio. >> >> This partially reverts commit 12fb0ac05, which was correct >> on the mailing list but got corrupted by the maintainer :p >> >> Introduced-by: 3b876140-c035-dd39-75d0-d54c48128...@redhat.com >> Reported-by: BALATON Zoltan >> Suggested-by: Thomas Huth >> Tested-by: Laurent Desnogues >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> See: >> http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06202.html (bug) >> http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg01309.html >> (report) >> >> Peter, Can this enters directly as bug-fix? >> >> chardev/char-stdio.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c >> index d83e60e787..96375f2ab8 100644 >> --- a/chardev/char-stdio.c >> +++ b/chardev/char-stdio.c >> @@ -59,7 +59,7 @@ static void qemu_chr_set_echo_stdio(Chardev *chr, bool >> echo) >> if (!echo) { >> tty.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP >> | INLCR | IGNCR | ICRNL | IXON); >> -tty.c_oflag &= ~OPOST; >> +tty.c_oflag |= OPOST; >> tty.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN); >> tty.c_cflag &= ~(CSIZE | PARENB); >> tty.c_cflag |= CS8; >> > > I think this is the right way to go. > > Reviewed-by: Thomas Huth Applied to master, thanks. -- PMM
Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
On 07.06.2018 23:08, Philippe Mathieu-Daudé wrote: > Remove the 'stair-step output' on stdio. > > This partially reverts commit 12fb0ac05, which was correct > on the mailing list but got corrupted by the maintainer :p > > Introduced-by: 3b876140-c035-dd39-75d0-d54c48128...@redhat.com > Reported-by: BALATON Zoltan > Suggested-by: Thomas Huth > Tested-by: Laurent Desnogues > Signed-off-by: Philippe Mathieu-Daudé > --- > See: > http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06202.html (bug) > http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg01309.html (report) > > Peter, Can this enters directly as bug-fix? > > chardev/char-stdio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c > index d83e60e787..96375f2ab8 100644 > --- a/chardev/char-stdio.c > +++ b/chardev/char-stdio.c > @@ -59,7 +59,7 @@ static void qemu_chr_set_echo_stdio(Chardev *chr, bool echo) > if (!echo) { > tty.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP > | INLCR | IGNCR | ICRNL | IXON); > -tty.c_oflag &= ~OPOST; > +tty.c_oflag |= OPOST; > tty.c_lflag &= ~(ECHO | ECHONL | ICANON | IEXTEN); > tty.c_cflag &= ~(CSIZE | PARENB); > tty.c_cflag |= CS8; > I think this is the right way to go. Reviewed-by: Thomas Huth