* [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial
@ 2018-05-23 19:50 Patryk Olszewski
2018-05-24 5:36 ` Thomas Huth
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Patryk Olszewski @ 2018-05-23 19:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Marc-André Lureau, thuth, Patryk Olszewski
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) {
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial
2018-05-23 19:50 [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial Patryk Olszewski
@ 2018-05-24 5:36 ` Thomas Huth
2018-05-28 15:54 ` Paolo Bonzini
2018-05-24 6:15 ` Markus Armbruster
2018-05-28 15:56 ` Paolo Bonzini
2 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2018-05-24 5:36 UTC (permalink / raw)
To: Patryk Olszewski, qemu-devel; +Cc: Paolo Bonzini, Marc-André Lureau
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial
2018-05-24 5:36 ` Thomas Huth
@ 2018-05-28 15:54 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-05-28 15:54 UTC (permalink / raw)
To: Thomas Huth, Patryk Olszewski, qemu-devel; +Cc: Marc-André Lureau
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial
2018-05-23 19:50 [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial Patryk Olszewski
2018-05-24 5:36 ` Thomas Huth
@ 2018-05-24 6:15 ` Markus Armbruster
2018-05-28 15:56 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2018-05-24 6:15 UTC (permalink / raw)
To: Patryk Olszewski; +Cc: qemu-devel, Paolo Bonzini, thuth, Marc-André Lureau
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial
2018-05-23 19:50 [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial Patryk Olszewski
2018-05-24 5:36 ` Thomas Huth
2018-05-24 6:15 ` Markus Armbruster
@ 2018-05-28 15:56 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-05-28 15:56 UTC (permalink / raw)
To: Patryk Olszewski, qemu-devel; +Cc: Marc-André Lureau, thuth
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-28 15:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-23 19:50 [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial Patryk Olszewski
2018-05-24 5:36 ` Thomas Huth
2018-05-28 15:54 ` Paolo Bonzini
2018-05-24 6:15 ` Markus Armbruster
2018-05-28 15:56 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).