Message ID | 20180607210818.12727-1-f4bug@amsat.org |
---|---|
State | Accepted, archived |
Headers | show |
Series | chardev: Restore CR,LF on stdio | expand |
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-d54c48128fac@redhat.com > Reported-by: BALATON Zoltan <balaton@eik.bme.hu> > Suggested-by: Thomas Huth <thuth@redhat.com> > Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > 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 <thuth@redhat.com>
On 8 June 2018 at 06:47, Thomas Huth <thuth@redhat.com> 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-d54c48128fac@redhat.com >> Reported-by: BALATON Zoltan <balaton@eik.bme.hu> >> Suggested-by: Thomas Huth <thuth@redhat.com> >> Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> 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 <thuth@redhat.com> Applied to master, thanks. -- PMM
W dniu 08.06.2018 o 17:25, Peter Maydell pisze: > On 8 June 2018 at 06:47, Thomas Huth <thuth@redhat.com> 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-d54c48128fac@redhat.com >>> Reported-by: BALATON Zoltan <balaton@eik.bme.hu> >>> Suggested-by: Thomas Huth <thuth@redhat.com> >>> Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> --- >>> 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 <thuth@redhat.com> > 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.
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 <thuth@redhat.com> 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-d54c48128fac@redhat.com >>>> Reported-by: BALATON Zoltan <balaton@eik.bme.hu> >>>> Suggested-by: Thomas Huth <thuth@redhat.com> >>>> Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> >>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>>> --- >>>> 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 <thuth@redhat.com> >> 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
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;