pty: using tcgetattr() to get attributes before re-setting it

Message ID 20180528103936.22538-1-liwang@redhat.com
State New
Headers show
Series
  • pty: using tcgetattr() to get attributes before re-setting it
Related show

Commit Message

Li Wang May 28, 2018, 10:39 a.m.
To fix this error:
  tst_test.c:1015: INFO: Timeout per run is 0h 50m 00s
  pty02.c:42: BROK: tcsetattr() failed: EINVAL

POSIX.1 General description:
  Changes the attributes associated with a terminal. New attributes are
  specified with a termios control structure. Programs should always
  issue a tcgetattr() first, modify the desired fields, and then issue
  a tcsetattr(). tcsetattr() should never be issued using a termios
  structure that was not obtained using tcgetattr(). tcsetattr() should
  use only a termios structure that was obtained by tcgetattr().

Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Eric Biggers <ebiggers@google.com>
Cc: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/pty/pty02.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Xiao Yang May 29, 2018, 7:03 a.m. | #1
Hi Li,

We found other issues by running pty02:
1) The undefined EXTPROC flag led to complier error on RHEL5/6.
2) Based on the fix, reading pts blocked on RHEL6 because we didn't write newline('\n') to ptmx,
    but it worked well on RHEL7.  I am not sure which kernel patch changed this behavior.
    According canonical mode description:
        In canonical mode:
        * Input is made available line by line.  An input line is available when one of the line
          delimiters is typed (NL, EOL, EOL2; or EOF at the start of line).  Except in the case of EOF,
          the line delimiter is included in the buffer returned by read(2).
3) Based on the fix, tcsetattr(3) cannot detect invalid EXTPROC flag on RHEL5/6.

Thanks,
Xiao Yang

On 2018/05/28 18:39, Li Wang wrote:
> To fix this error:
>    tst_test.c:1015: INFO: Timeout per run is 0h 50m 00s
>    pty02.c:42: BROK: tcsetattr() failed: EINVAL
>
> POSIX.1 General description:
>    Changes the attributes associated with a terminal. New attributes are
>    specified with a termios control structure. Programs should always
>    issue a tcgetattr() first, modify the desired fields, and then issue
>    a tcsetattr(). tcsetattr() should never be issued using a termios
>    structure that was not obtained using tcgetattr(). tcsetattr() should
>    use only a termios structure that was obtained by tcgetattr().
>
> Signed-off-by: Li Wang<liwang@redhat.com>
> Cc: Eric Biggers<ebiggers@google.com>
> Cc: Cyril Hrubis<chrubis@suse.cz>
> ---
>   testcases/kernel/pty/pty02.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/pty/pty02.c b/testcases/kernel/pty/pty02.c
> index fd3d26b..0cedca8 100644
> --- a/testcases/kernel/pty/pty02.c
> +++ b/testcases/kernel/pty/pty02.c
> @@ -31,13 +31,18 @@
>
>   static void do_test(void)
>   {
> -	struct termios io = { .c_lflag = EXTPROC | ICANON };
> +	struct termios io;
>   	int ptmx, pts;
>   	char c = 'A';
>   	int nbytes;
>
>   	ptmx = SAFE_OPEN("/dev/ptmx", O_WRONLY);
>
> +	if (tcgetattr(ptmx,&io) != 0)
> +		tst_brk(TBROK | TERRNO, "tcgetattr() failed");
> +
> +	io.c_lflag = EXTPROC | ICANON;
> +
>   	if (tcsetattr(ptmx, TCSANOW,&io) != 0)
>   		tst_brk(TBROK | TERRNO, "tcsetattr() failed");
>
Li Wang May 30, 2018, 9:52 a.m. | #2
Hi Xiao,

On Tue, May 29, 2018 at 3:03 PM, Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:

> Hi Li,
>
> We found other issues by running pty02:
> 1) The undefined EXTPROC flag led to complier error on RHEL5/6.
> 2) Based on the fix, reading pts blocked on RHEL6 because we didn't write
> newline('\n') to ptmx,
>    but it worked well on RHEL7.  I am not sure which kernel patch changed
> this behavior.
>    According canonical mode description:
>        In canonical mode:
>        * Input is made available line by line.  An input line is available
> when one of the line
>          delimiters is typed (NL, EOL, EOL2; or EOF at the start of
> line).  Except in the case of EOF,
>          the line delimiter is included in the buffer returned by read(2).
> 3) Based on the fix,
> ​​
> tcsetattr(3) cannot detect invalid EXTPROC flag on RHEL5/6.
>

AFAIK, values of the c_lflag field describe the control of various
functions in struct termios,
but it not defined in POSIX and not supported under all Linux. So
​
tcsetattr() cannot detect valid EXTPROC on RHEL5/6 is acceptable I think.
Also, the ICANON has different behavior on some kind of UNIX OS too.

To solve these problem, maybe we have to define the EXTPROC in lapi, and
skip this test on kernel without that support.

That's what I can think of, if anything wrong, plz correct me.

Thanks,
Li Wang
<div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Xiao,<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 29, 2018 at 3:03 PM, Xiao Yang <span dir="ltr">&lt;<a href="mailto:yangx.jy@cn.fujitsu.com" target="_blank">yangx.jy@cn.fujitsu.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Li,<br>
<br>
We found other issues by running pty02:<br>
1) The undefined EXTPROC flag led to complier error on RHEL5/6.<br>
2) Based on the fix, reading pts blocked on RHEL6 because we didn&#39;t write newline(&#39;\n&#39;) to ptmx,<br>
   but it worked well on RHEL7.  I am not sure which kernel patch changed this behavior.<br>
   According canonical mode description:<br>
       In canonical mode:<br>
       * Input is made available line by line.  An input line is available when one of the line<br>
         delimiters is typed (NL, EOL, EOL2; or EOF at the start of line).  Except in the case of EOF,<br>
         the line delimiter is included in the buffer returned by read(2).<br>
3) Based on the fix, <div style="font-size:small;display:inline" class="gmail_default">​​</div>tcsetattr(3) cannot detect invalid EXTPROC flag on RHEL5/6.<br></blockquote><div><br></div><div style="font-size:small" class="gmail_default">AFAIK, values of the c_lflag field describe the control of various functions in struct termios,</div><div style="font-size:small" class="gmail_default">but it not defined in POSIX and not supported under all Linux. So <div style="font-size:small;display:inline" class="gmail_default">​</div>tcsetattr() cannot detect valid EXTPROC on RHEL5/6 is acceptable I think. Also, the ICANON has different behavior on some kind of UNIX OS too.<br><font size="2"><span style="font-family:arial,helvetica,sans-serif"></span></font><font size="2"><span style="font-family:arial,helvetica,sans-serif"></span></font></div><div style="font-size:small" class="gmail_default"><br></div><div style="font-size:small" class="gmail_default"><font size="2">To solve these problem, maybe we have to define the EXTPROC in lapi, and skip this test on kernel without that support.</font></div><div style="font-size:small" class="gmail_default"><font size="2"><br></font></div><div style="font-size:small" class="gmail_default"><font size="2">That&#39;s what I can think of, if anything wrong, plz correct me.</font></div><div style="font-size:small" class="gmail_default"><font size="2"><br></font></div><div style="font-size:small" class="gmail_default"><font size="2">Thanks,</font></div><div style="font-size:small" class="gmail_default">Li Wang<br></div></div></div></div>

Patch

diff --git a/testcases/kernel/pty/pty02.c b/testcases/kernel/pty/pty02.c
index fd3d26b..0cedca8 100644
--- a/testcases/kernel/pty/pty02.c
+++ b/testcases/kernel/pty/pty02.c
@@ -31,13 +31,18 @@ 
 
 static void do_test(void)
 {
-	struct termios io = { .c_lflag = EXTPROC | ICANON };
+	struct termios io;
 	int ptmx, pts;
 	char c = 'A';
 	int nbytes;
 
 	ptmx = SAFE_OPEN("/dev/ptmx", O_WRONLY);
 
+	if (tcgetattr(ptmx, &io) != 0)
+		tst_brk(TBROK | TERRNO, "tcgetattr() failed");
+
+	io.c_lflag = EXTPROC | ICANON;
+
 	if (tcsetattr(ptmx, TCSANOW, &io) != 0)
 		tst_brk(TBROK | TERRNO, "tcsetattr() failed");