qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
@ 2018-06-07 21:08 Philippe Mathieu-Daudé
  2018-06-08  5:47 ` Thomas Huth
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-06-07 21:08 UTC (permalink / raw)
  To: Peter Maydell, Paolo Bonzini
  Cc: Philippe Mathieu-Daudé, Patryk Olszewski, BALATON Zoltan,
	Marc-André Lureau, qemu-devel, Thomas Huth,
	Markus Armbruster, Eric Blake, Laurent Desnogues

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;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
  2018-06-07 21:08 [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio Philippe Mathieu-Daudé
@ 2018-06-08  5:47 ` Thomas Huth
  2018-06-08 15:25   ` Peter Maydell
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Huth @ 2018-06-08  5:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Peter Maydell, Paolo Bonzini
  Cc: Patryk Olszewski, BALATON Zoltan, Marc-André Lureau,
	qemu-devel, Markus Armbruster, Eric Blake, Laurent Desnogues,
	QEMU Trivial

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>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
  2018-06-08  5:47 ` Thomas Huth
@ 2018-06-08 15:25   ` Peter Maydell
  2018-06-08 15:58     ` Patryk Olszewski
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2018-06-08 15:25 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Philippe Mathieu-Daudé, Paolo Bonzini, Patryk Olszewski,
	BALATON Zoltan, Marc-André Lureau, QEMU Developers,
	Markus Armbruster, Eric Blake, Laurent Desnogues, QEMU Trivial

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
  2018-06-08 15:25   ` Peter Maydell
@ 2018-06-08 15:58     ` Patryk Olszewski
  2018-06-08 20:10       ` Thomas Huth
  0 siblings, 1 reply; 5+ messages in thread
From: Patryk Olszewski @ 2018-06-08 15:58 UTC (permalink / raw)
  To: Peter Maydell, Thomas Huth
  Cc: Philippe Mathieu-Daudé, Paolo Bonzini, BALATON Zoltan,
	Marc-André Lureau, QEMU Developers, Markus Armbruster,
	Eric Blake, Laurent Desnogues, QEMU Trivial

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio
  2018-06-08 15:58     ` Patryk Olszewski
@ 2018-06-08 20:10       ` Thomas Huth
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2018-06-08 20:10 UTC (permalink / raw)
  To: Patryk Olszewski, Peter Maydell
  Cc: QEMU Trivial, QEMU Developers, Markus Armbruster,
	Marc-André Lureau, Laurent Desnogues, Paolo Bonzini,
	Philippe Mathieu-Daudé

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-06-08 20:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-07 21:08 [Qemu-devel] [PATCH] chardev: Restore CR,LF on stdio Philippe Mathieu-Daudé
2018-06-08  5:47 ` Thomas Huth
2018-06-08 15:25   ` Peter Maydell
2018-06-08 15:58     ` Patryk Olszewski
2018-06-08 20:10       ` Thomas Huth

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).