| Message ID | 1527105041-21013-1-git-send-email-patryk@fala.ehost.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Remove unwanted crlf conversion in serial | expand |
On 23.05.2018 21:50, Patryk Olszewski wrote: > This patch fixes bug in serial that made it almost impossible for guest > to communicate with devices through host's serial. > > OPOST flag in c_oflag enables output processing letting other flags in > c_oflag take effect. Usually in c_oflag ONLCR flag is also set, which > causes crlf to be sent in place of lf. This breaks binary transmissions. > Unsetting OPOST flag turns off any output processing which fixes the bug. > > Bug reports related: > https://bugs.launchpad.net/qemu/+bug/1772086 > https://bugs.launchpad.net/qemu/+bug/1407813 > https://bugs.launchpad.net/qemu/+bug/1715296 > also > https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html > > Signed-off-by: Patryk Olszewski <patryk@fala.ehost.pl> > --- > chardev/char-serial.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-serial.c b/chardev/char-serial.c > index feb52e5..ae548d2 100644 > --- a/chardev/char-serial.c > +++ b/chardev/char-serial.c > @@ -139,7 +139,7 @@ static void tty_serial_init(int fd, int speed, > > 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 | ISIG); > tty.c_cflag &= ~(CSIZE | PARENB | PARODD | CRTSCTS | CSTOPB); > switch (data_bits) { > I think this bug has good chances to win the price "Oldest bug of the year". If we'd wait one more months, it has its 15th anniversary: The code has originally been added in 2003, see commit 0824d6fc674084519c85, it has just been moved around to other files afterwards. Thus this is almost as old as QEMU itself! Anyway, I think disabling output processing is the right thing to do here, so: Reviewed-by: Thomas Huth <thuth@redhat.com>
Patryk Olszewski <patryk@fala.ehost.pl> writes: > This patch fixes bug in serial that made it almost impossible for guest > to communicate with devices through host's serial. Well, "almost impossible" for some applications. Text works fine. But I get what you mean. > OPOST flag in c_oflag enables output processing letting other flags in > c_oflag take effect. Usually in c_oflag ONLCR flag is also set, which > causes crlf to be sent in place of lf. This breaks binary transmissions. > Unsetting OPOST flag turns off any output processing which fixes the bug. > > Bug reports related: > https://bugs.launchpad.net/qemu/+bug/1772086 > https://bugs.launchpad.net/qemu/+bug/1407813 > https://bugs.launchpad.net/qemu/+bug/1715296 > also > https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html > > Signed-off-by: Patryk Olszewski <patryk@fala.ehost.pl> > --- > chardev/char-serial.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-serial.c b/chardev/char-serial.c > index feb52e5..ae548d2 100644 > --- a/chardev/char-serial.c > +++ b/chardev/char-serial.c > @@ -139,7 +139,7 @@ static void tty_serial_init(int fd, int speed, > > 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 | ISIG); > tty.c_cflag &= ~(CSIZE | PARENB | PARODD | CRTSCTS | CSTOPB); > switch (data_bits) { Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 24/05/2018 07:36, Thomas Huth wrote: > On 23.05.2018 21:50, Patryk Olszewski wrote: >> This patch fixes bug in serial that made it almost impossible for guest >> to communicate with devices through host's serial. >> >> OPOST flag in c_oflag enables output processing letting other flags in >> c_oflag take effect. Usually in c_oflag ONLCR flag is also set, which >> causes crlf to be sent in place of lf. This breaks binary transmissions. >> Unsetting OPOST flag turns off any output processing which fixes the bug. >> >> Bug reports related: >> https://bugs.launchpad.net/qemu/+bug/1772086 >> https://bugs.launchpad.net/qemu/+bug/1407813 >> https://bugs.launchpad.net/qemu/+bug/1715296 >> also >> https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html >> >> Signed-off-by: Patryk Olszewski <patryk@fala.ehost.pl> >> --- >> chardev/char-serial.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/chardev/char-serial.c b/chardev/char-serial.c >> index feb52e5..ae548d2 100644 >> --- a/chardev/char-serial.c >> +++ b/chardev/char-serial.c >> @@ -139,7 +139,7 @@ static void tty_serial_init(int fd, int speed, >> >> 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 | ISIG); >> tty.c_cflag &= ~(CSIZE | PARENB | PARODD | CRTSCTS | CSTOPB); >> switch (data_bits) { >> > > I think this bug has good chances to win the price "Oldest bug of the > year". If we'd wait one more months, it has its 15th anniversary: The > code has originally been added in 2003, see commit 0824d6fc674084519c85, > it has just been moved around to other files afterwards. Thus this is > almost as old as QEMU itself! It was also already fixed once in commit 64b7b7334b92c1fdc1bb7d3d1afc342656c59539 Author: aurel32 <aurel32@c046a42c-6fe2-441c-8c8c-71466251a162> Date: Mon May 5 10:05:31 2008 +0000 Put Pseudo-TTY in rawmode for char devices (Daniel P. Berrange) Queued, thanks. Paolo
On 23/05/2018 21:50, Patryk Olszewski wrote: > This patch fixes bug in serial that made it almost impossible for guest > to communicate with devices through host's serial. > > OPOST flag in c_oflag enables output processing letting other flags in > c_oflag take effect. Usually in c_oflag ONLCR flag is also set, which > causes crlf to be sent in place of lf. This breaks binary transmissions. > Unsetting OPOST flag turns off any output processing which fixes the bug. > > Bug reports related: > https://bugs.launchpad.net/qemu/+bug/1772086 > https://bugs.launchpad.net/qemu/+bug/1407813 > https://bugs.launchpad.net/qemu/+bug/1715296 > also > https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html > > Signed-off-by: Patryk Olszewski <patryk@fala.ehost.pl> > --- > chardev/char-serial.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-serial.c b/chardev/char-serial.c > index feb52e5..ae548d2 100644 > --- a/chardev/char-serial.c > +++ b/chardev/char-serial.c > @@ -139,7 +139,7 @@ static void tty_serial_init(int fd, int speed, > > 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 | ISIG); > tty.c_cflag &= ~(CSIZE | PARENB | PARODD | CRTSCTS | CSTOPB); > switch (data_bits) { > The same code was also copied in chardev/char-stdio.c, I fixed up that one too. Paolo
diff --git a/chardev/char-serial.c b/chardev/char-serial.c index feb52e5..ae548d2 100644 --- a/chardev/char-serial.c +++ b/chardev/char-serial.c @@ -139,7 +139,7 @@ static void tty_serial_init(int fd, int speed, 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 | ISIG); tty.c_cflag &= ~(CSIZE | PARENB | PARODD | CRTSCTS | CSTOPB); switch (data_bits) {
This patch fixes bug in serial that made it almost impossible for guest to communicate with devices through host's serial. OPOST flag in c_oflag enables output processing letting other flags in c_oflag take effect. Usually in c_oflag ONLCR flag is also set, which causes crlf to be sent in place of lf. This breaks binary transmissions. Unsetting OPOST flag turns off any output processing which fixes the bug. Bug reports related: https://bugs.launchpad.net/qemu/+bug/1772086 https://bugs.launchpad.net/qemu/+bug/1407813 https://bugs.launchpad.net/qemu/+bug/1715296 also https://lists.nongnu.org/archive/html/qemu-devel/2006-06/msg00196.html Signed-off-by: Patryk Olszewski <patryk@fala.ehost.pl> --- chardev/char-serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)