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

2018-06-08 Thread Thomas Huth 
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  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-d54c48128...@redhat.com
 Reported-by: BALATON Zoltan 
 Suggested-by: Thomas Huth 
 Tested-by: Laurent Desnogues 
 Signed-off-by: Philippe Mathieu-Daudé 
 ---
 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 
>> 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

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

2018-06-08 Thread Patryk Olszewski 
W dniu 08.06.2018 o 17:25, Peter Maydell pisze:
> On 8 June 2018 at 06:47, Thomas Huth  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-d54c48128...@redhat.com
>>> Reported-by: BALATON Zoltan 
>>> Suggested-by: Thomas Huth 
>>> Tested-by: Laurent Desnogues 
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>> ---
>>> 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 
> 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.

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

2018-06-08 Thread Peter Maydell 
On 8 June 2018 at 06:47, Thomas Huth  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-d54c48128...@redhat.com
>> Reported-by: BALATON Zoltan 
>> Suggested-by: Thomas Huth 
>> Tested-by: Laurent Desnogues 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> 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 

Applied to master, thanks.

-- PMM

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

2018-06-07 Thread Thomas Huth 
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-d54c48128...@redhat.com
> Reported-by: BALATON Zoltan 
> Suggested-by: Thomas Huth 
> Tested-by: Laurent Desnogues 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> 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