Re: [Qemu-devel] [PATCH v2] Remove unwanted crlf conversion in serial

[Thomas Huth]

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 <address@hidden>
> ---
>  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 <address@hidden>