Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio

[Thomas Huth <thuth@redhat.com>]

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