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

[Patryk Olszewski]

W dniu 23.05.2018 o 18:40, Markus Armbruster pisze:
> Patryk <address@hidden> writes:
>
>> W dniu 22.05.2018 o 15:07, Peter Maydell pisze:
>>> On 22 May 2018 at 12:53, Markus Armbruster <address@hidden> wrote:
>>>> Patryk Olszewski <address@hidden> writes:
>>>>
>>>>> 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) {
>>>> This change may well make sense, but your commit message needs to
>>>> explain *why*.
>>>>
>>>> For what it's worth, POSIX documents OPOST as "Post-process output", and
>>>> the Linux manual page as "Enable implementation-defined output
>>>> processing."
>>> We've set OPOST on our terminals since forever, right back to
>>> commit 0824d6fc674 in 2003. Not setting it seems like the right thing,
>>> though, since we're generally otherwise setting the thing up to be a
>>> raw mode tty (and if we're connecting this to a guest then raw seems
>>> like what we want).
>>>
>>> I wonder whether connecting, say, the HMP monitor to a 'serial'
>>> chardev (a) works now (b) ends up with stair-step output after
>>> this change (c) is something we care about...
>>>
>>> thanks
>>> -- PMM
>>>
>> This patch is here to help fix years old bug of lf being replaced with
>> crlf in serial, which is super problematic in binary transmissions,
>> making communication with devices through serial from guest almost
>> impossible.
>>
>> Setting OPOST flag in c_oflag enables the output processing, in other
>> words it makes any other flag set in c_oflag come into action. From my
>> quick experiment on serial devices on Linux I found out that by default
>> c_oflag has enabled ONLCR flag, which is the one responsible for the
>> crlf conversion. Unsetting OPOST disables it.
>>
>> Bug reports related to that:
>>
>> 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
> Work that into your commit message, and you got a fine patch as far as
> I'm concerned :)
>
First time contribution to such large project. Trying to learn how to
properly work with mailing lists and such. If there's something I
actually have to do now I'd like to get some pointers on how to do it.
Thank you,