Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE [BZ #10339]

  struct termios2 is required for setting arbitrary baud rates on serial
ports.  <sys/ioctl.h> and <linux/termios.h> have conflicting
definitions in the existing termios definitions, which means that it
is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
with glibc.  Providing a definition within glibc resolves this problem.

This does not completely address bug 10339, but it at least exposes
the current kernel functionality in this area.

Support for struct termios2 is architecture-specific in the kernel.
Support on alpha was only added in Linux 4.20.  POWER support is
currently missing.  The expectation is that the kernel will eventually
use the generic UAPI definition for struct termios2.

2019-04-09  Florian Weimer  <fweimer@redhat.com>

	[BZ #10339]
	Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE.
	* sysdeps/unix/sysv/linux/Makefile [$(subdir) == termios] (tests):
	Add tst-termios2.
	* sysdeps/unix/sysv/linux/tst-termios2.c: New file.
	* sysdeps/unix/sysv/linux/bits/termios2-struct.h: Likewise.
	* sysdeps/unix/sysv/linux/bits/termios.h [__USE_GNU]: Include it.
	* sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h: New file.
	* sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h: Likewise.
  

On April 9, 2019 4:47:30 AM MDT, Florian Weimer <fweimer@redhat.com> wrote:
>struct termios2 is required for setting arbitrary baud rates on serial
>ports.  <sys/ioctl.h> and <linux/termios.h> have conflicting
>definitions in the existing termios definitions, which means that it
>is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
>with glibc.  Providing a definition within glibc resolves this problem.
>
>This does not completely address bug 10339, but it at least exposes
>the current kernel functionality in this area.
>
>Support for struct termios2 is architecture-specific in the kernel.
>Support on alpha was only added in Linux 4.20.  POWER support is
>currently missing.  The expectation is that the kernel will eventually
>use the generic UAPI definition for struct termios2.
>
>2019-04-09  Florian Weimer  <fweimer@redhat.com>
>
>	[BZ #10339]
>	Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE.
>	* sysdeps/unix/sysv/linux/Makefile [$(subdir) == termios] (tests):
>	Add tst-termios2.
>	* sysdeps/unix/sysv/linux/tst-termios2.c: New file.
>	* sysdeps/unix/sysv/linux/bits/termios2-struct.h: Likewise.
>	* sysdeps/unix/sysv/linux/bits/termios.h [__USE_GNU]: Include it.
>	* sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h: New file.
>	* sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h: Likewise.
>
>diff --git a/NEWS b/NEWS
>index b58e2469d4..5e6ecb9c7d 100644
>--- a/NEWS
>+++ b/NEWS
>@@ -18,6 +18,9 @@ Major new features:
> 
> * On Linux, the gettid function has been added.
> 
>+* On Linux, <termios.h> now provides a definition of struct termios2
>with
>+  the _GNU_SOURCE feature test macro.
>+
> * Minguo (Republic of China) calendar support has been added as an
> alternative calendar for the following locales: zh_TW, cmn_TW, hak_TW,
>   nan_TW, lzh_TW.
>diff --git a/sysdeps/unix/sysv/linux/Makefile
>b/sysdeps/unix/sysv/linux/Makefile
>index 52ac6ad484..4cb5e4f0d2 100644
>--- a/sysdeps/unix/sysv/linux/Makefile
>+++ b/sysdeps/unix/sysv/linux/Makefile
>@@ -156,6 +156,7 @@ endif
> 
> ifeq ($(subdir),termios)
> sysdep_headers += termio.h
>+tests += tst-termios2
> endif
> 
> ifeq ($(subdir),posix)
>diff --git a/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5f09445e23
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition.  Linux/alpha version.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+  tcflag_t c_iflag;
>+  tcflag_t c_oflag;
>+  tcflag_t c_cflag;
>+  tcflag_t c_lflag;
>+  cc_t c_cc[NCCS];
>+  cc_t c_line;
>+  speed_t c_ispeed;
>+  speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/bits/termios.h
>b/sysdeps/unix/sysv/linux/bits/termios.h
>index 997231cd03..45ac7affdf 100644
>--- a/sysdeps/unix/sysv/linux/bits/termios.h
>+++ b/sysdeps/unix/sysv/linux/bits/termios.h
>@@ -25,6 +25,10 @@ typedef unsigned int	speed_t;
> typedef unsigned int	tcflag_t;
> 
> #include <bits/termios-struct.h>
>+#ifdef __USE_GNU
>+# include <bits/termios2-struct.h>
>+#endif
>+
> #include <bits/termios-c_cc.h>
> #include <bits/termios-c_iflag.h>
> #include <bits/termios-c_oflag.h>
>diff --git a/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5a48e45ef3
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition.  Linux/generic version.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+  tcflag_t c_iflag;
>+  tcflag_t c_oflag;
>+  tcflag_t c_cflag;
>+  tcflag_t c_lflag;
>+  cc_t c_line;
>+  cc_t c_cc[NCCS];
>+  speed_t c_ispeed;
>+  speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..7c889e575c
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition.  Linux/sparc version.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+  tcflag_t c_iflag;
>+  tcflag_t c_oflag;
>+  tcflag_t c_cflag;
>+  tcflag_t c_lflag;
>+  cc_t c_line;
>+  cc_t c_cc[NCCS + 2];
>+  speed_t c_ispeed;
>+  speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/tst-termios2.c
>b/sysdeps/unix/sysv/linux/tst-termios2.c
>new file mode 100644
>index 0000000000..82326a1288
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/tst-termios2.c
>@@ -0,0 +1,48 @@
>+/* Minimal test of struct termios2 definition.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#include <termios.h>
>+#include <sys/ioctl.h>
>+
>+/* This function is never executed, but must be compiled successfully.
>+   Accessing serial ports in the test suite is problematic because
>+   they likely correspond with low-level system functionality.  */
>+void
>+not_executed (int fd)
>+{
>+  /* Avoid a compilation failure if TCGETS2, TCSETS2 are not
>+     defined.  */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+  struct termios2 ti;
>+  ioctl (fd, TCGETS2, &ti);
>+  ioctl (fd, TCSETS2, &ti);
>+#endif
>+}
>+
>+static int
>+do_test (void)
>+{
>+  /* Fail at run time if TCGETS2 or TCSETS2 is not defined.  */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+  return 0;
>+#else
>+  return 1;
>+#endif
>+}
>+
>+#include <support/test-driver.c>

PowerPC doesn't need it at all.
  

On April 9, 2019 4:47:30 AM MDT, Florian Weimer <fweimer@redhat.com> wrote:
>struct termios2 is required for setting arbitrary baud rates on serial
>ports.  <sys/ioctl.h> and <linux/termios.h> have conflicting
>definitions in the existing termios definitions, which means that it
>is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
>with glibc.  Providing a definition within glibc resolves this problem.
>
>This does not completely address bug 10339, but it at least exposes
>the current kernel functionality in this area.
>
>Support for struct termios2 is architecture-specific in the kernel.
>Support on alpha was only added in Linux 4.20.  POWER support is
>currently missing.  The expectation is that the kernel will eventually
>use the generic UAPI definition for struct termios2.
>
>2019-04-09  Florian Weimer  <fweimer@redhat.com>
>
>	[BZ #10339]
>	Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE.
>	* sysdeps/unix/sysv/linux/Makefile [$(subdir) == termios] (tests):
>	Add tst-termios2.
>	* sysdeps/unix/sysv/linux/tst-termios2.c: New file.
>	* sysdeps/unix/sysv/linux/bits/termios2-struct.h: Likewise.
>	* sysdeps/unix/sysv/linux/bits/termios.h [__USE_GNU]: Include it.
>	* sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h: New file.
>	* sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h: Likewise.
>
>diff --git a/NEWS b/NEWS
>index b58e2469d4..5e6ecb9c7d 100644
>--- a/NEWS
>+++ b/NEWS
>@@ -18,6 +18,9 @@ Major new features:
> 
> * On Linux, the gettid function has been added.
> 
>+* On Linux, <termios.h> now provides a definition of struct termios2
>with
>+  the _GNU_SOURCE feature test macro.
>+
> * Minguo (Republic of China) calendar support has been added as an
> alternative calendar for the following locales: zh_TW, cmn_TW, hak_TW,
>   nan_TW, lzh_TW.
>diff --git a/sysdeps/unix/sysv/linux/Makefile
>b/sysdeps/unix/sysv/linux/Makefile
>index 52ac6ad484..4cb5e4f0d2 100644
>--- a/sysdeps/unix/sysv/linux/Makefile
>+++ b/sysdeps/unix/sysv/linux/Makefile
>@@ -156,6 +156,7 @@ endif
> 
> ifeq ($(subdir),termios)
> sysdep_headers += termio.h
>+tests += tst-termios2
> endif
> 
> ifeq ($(subdir),posix)
>diff --git a/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5f09445e23
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition.  Linux/alpha version.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+  tcflag_t c_iflag;
>+  tcflag_t c_oflag;
>+  tcflag_t c_cflag;
>+  tcflag_t c_lflag;
>+  cc_t c_cc[NCCS];
>+  cc_t c_line;
>+  speed_t c_ispeed;
>+  speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/bits/termios.h
>b/sysdeps/unix/sysv/linux/bits/termios.h
>index 997231cd03..45ac7affdf 100644
>--- a/sysdeps/unix/sysv/linux/bits/termios.h
>+++ b/sysdeps/unix/sysv/linux/bits/termios.h
>@@ -25,6 +25,10 @@ typedef unsigned int	speed_t;
> typedef unsigned int	tcflag_t;
> 
> #include <bits/termios-struct.h>
>+#ifdef __USE_GNU
>+# include <bits/termios2-struct.h>
>+#endif
>+
> #include <bits/termios-c_cc.h>
> #include <bits/termios-c_iflag.h>
> #include <bits/termios-c_oflag.h>
>diff --git a/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..5a48e45ef3
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition.  Linux/generic version.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+  tcflag_t c_iflag;
>+  tcflag_t c_oflag;
>+  tcflag_t c_cflag;
>+  tcflag_t c_lflag;
>+  cc_t c_line;
>+  cc_t c_cc[NCCS];
>+  speed_t c_ispeed;
>+  speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>new file mode 100644
>index 0000000000..7c889e575c
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
>@@ -0,0 +1,33 @@
>+/* struct termios2 definition.  Linux/sparc version.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#ifndef _TERMIOS_H
>+# error "Never include <bits/termios2-struct.h> directly; use
><termios.h> instead."
>+#endif
>+
>+struct termios2
>+{
>+  tcflag_t c_iflag;
>+  tcflag_t c_oflag;
>+  tcflag_t c_cflag;
>+  tcflag_t c_lflag;
>+  cc_t c_line;
>+  cc_t c_cc[NCCS + 2];
>+  speed_t c_ispeed;
>+  speed_t c_ospeed;
>+};
>diff --git a/sysdeps/unix/sysv/linux/tst-termios2.c
>b/sysdeps/unix/sysv/linux/tst-termios2.c
>new file mode 100644
>index 0000000000..82326a1288
>--- /dev/null
>+++ b/sysdeps/unix/sysv/linux/tst-termios2.c
>@@ -0,0 +1,48 @@
>+/* Minimal test of struct termios2 definition.
>+   Copyright (C) 2019 Free Software Foundation, Inc.
>+   This file is part of the GNU C Library.
>+
>+   The GNU C Library is free software; you can redistribute it and/or
>+   modify it under the terms of the GNU Lesser General Public
>+   License as published by the Free Software Foundation; either
>+   version 2.1 of the License, or (at your option) any later version.
>+
>+   The GNU C Library is distributed in the hope that it will be
>useful,
>+   but WITHOUT ANY WARRANTY; without even the implied warranty of
>+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+   Lesser General Public License for more details.
>+
>+   You should have received a copy of the GNU Lesser General Public
>+   License along with the GNU C Library.  If not, see
>+   <http://www.gnu.org/licenses/>.  */
>+
>+#include <termios.h>
>+#include <sys/ioctl.h>
>+
>+/* This function is never executed, but must be compiled successfully.
>+   Accessing serial ports in the test suite is problematic because
>+   they likely correspond with low-level system functionality.  */
>+void
>+not_executed (int fd)
>+{
>+  /* Avoid a compilation failure if TCGETS2, TCSETS2 are not
>+     defined.  */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+  struct termios2 ti;
>+  ioctl (fd, TCGETS2, &ti);
>+  ioctl (fd, TCSETS2, &ti);
>+#endif
>+}
>+
>+static int
>+do_test (void)
>+{
>+  /* Fail at run time if TCGETS2 or TCSETS2 is not defined.  */
>+#if defined (TCGETS2) && defined (TCSETS2)
>+  return 0;
>+#else
>+  return 1;
>+#endif
>+}
>+
>+#include <support/test-driver.c>

Anything blocking the POC code I provided?

Note: for PowerPC, define termios2 = kernel termios, and the ioctls as aliases.
  

* hpa:

> Anything blocking the POC code I provided?

I don't recall that, sorry.  Would you please provide a reference?

Thanks,
Florian
  

On Tue, Apr 9, 2019 at 6:48 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> struct termios2 is required for setting arbitrary baud rates on serial
> ports.  <sys/ioctl.h> and <linux/termios.h> have conflicting
> definitions in the existing termios definitions, which means that it
> is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
> with glibc.  Providing a definition within glibc resolves this problem.

This seems like a reasonable approach, but I am not familiar with
termios-related issues so this is not a full review.  I have one
technical note:

> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
> @@ -0,0 +1,33 @@
> +/* struct termios2 definition.  Linux/alpha version.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _TERMIOS_H
> +# error "Never include <bits/termios2-struct.h> directly; use <termios.h> instead."
> +#endif

All new installed headers should have multiple inclusion guards, even
if they are bits headers only intended to be included by one parent
header.  This removes noise from the "Multiple include guards may be
useful for:" diagnostic produced by gcc -H, and prevents e.g.

    #include <termios.h>
    #include <bits/termios2-struct.h>

from spewing unhelpful errors (yes, the above is incorrect, but we
should still try to avoid diagnostic spew).

zw
  

* Zack Weinberg:

> All new installed headers should have multiple inclusion guards, even
> if they are bits headers only intended to be included by one parent
> header.  This removes noise from the "Multiple include guards may be
> useful for:" diagnostic produced by gcc -H, and prevents e.g.
>
>     #include <termios.h>
>     #include <bits/termios2-struct.h>
>
> from spewing unhelpful errors (yes, the above is incorrect, but we
> should still try to avoid diagnostic spew).

Have there been any patches that follow this practice?  It's the first
time I hear about it.

Thanks,
Florian
  

On Tue, Apr 9, 2019 at 12:34 PM Florian Weimer <fweimer@redhat.com> wrote:
> * Zack Weinberg:
>
> > All new installed headers should have multiple inclusion guards, even
> > if they are bits headers only intended to be included by one parent
> > header.  This removes noise from the "Multiple include guards may be
> > useful for:" diagnostic produced by gcc -H, and prevents e.g.
> >
> >     #include <termios.h>
> >     #include <bits/termios2-struct.h>
> >
> > from spewing unhelpful errors (yes, the above is incorrect, but we
> > should still try to avoid diagnostic spew).
>
> Have there been any patches that follow this practice?  It's the first
> time I hear about it.

I thought this was standard practice for a long time now, but I could be wrong.

zw
  

* hpa:

> PowerPC doesn't need it at all.

Because the definition would be the same as struct termios?

I think we should still define struct termios2 using the struct termios
definition, and also define TCGETS2 and TCSETS2.  This way, applications
can use the struct termios2 type without affecting portability to POWER.

Thanks,
Florian
  

On 09/04/2019 07:47, Florian Weimer wrote:
> struct termios2 is required for setting arbitrary baud rates on serial
> ports.  <sys/ioctl.h> and <linux/termios.h> have conflicting
> definitions in the existing termios definitions, which means that it
> is currently very difficult to use TCGETS2/TCSETS2 and struct termios2
> with glibc.  Providing a definition within glibc resolves this problem.
> 
> This does not completely address bug 10339, but it at least exposes
> the current kernel functionality in this area.
> 
> Support for struct termios2 is architecture-specific in the kernel.
> Support on alpha was only added in Linux 4.20.  POWER support is
> currently missing.  The expectation is that the kernel will eventually
> use the generic UAPI definition for struct termios2.

I still think the better strategy, from both BZ#10339 and recent thread
discussion about the issue on libc-alpha, is rather to:

  1. Start to use termios2 ioctl kabi instead of termios1.  The only
     missing spot is alpha pre linux 4.20.

  2. Adjust sparc and mips to add c_ispeed and c_ospeed along with
     compat symbols to use termios1.  This will allow also some cleanup
     to remove _HAVE_STRUCT_TERMIOS_C_{O,I}SPEED.

  3. Use the compat symbols for alpha pre-4.20.

  4. With termios Linux ABI being essentially the same for all supported
     architectures (with support for c_ospeed and c_ispeed) we can move 
     forward to adapt the current cfgetospeed, cfgetispeed, cfsetospeed,
     cfsetispeed to work with arbitrary values.

     The POSIX and Linux extended BXX values will need to be handled 
     exceptionally.  It means their integers values will be reserved and
     mapped to the termios2 values.  The code resulting code for cfsetospeed,
     for instance, would be:

     ---
     static inline speed_t
     c_ispeed (tcflag_t c_cflag)
     {
       return (c_cflag >> IBSHIFT) & CBAUD;
     }

     /*
      * The top four bits in speed_t are reserved for future use, and *currently*
      * the equivalent values are the only valid baud_t values.
      */
     static inline bool         
     invalid_speed (speed_t x)
     {
       return x > 0x0fffffff;
     } 

     /* Set the output baud rate stored in *TERMIOS_P to the symbol SPEED */
     int
     cfsetospeed (struct termios *termios_p, speed_t speed)
     {
       if (invalid_speed (speed))
         {
           __set_errno (EINVAL); 
           return -1;
         }

       termios_p->c_ospeed = speed_kernel_from_user (speed);
       if ( c_ispeed (termios_p->c_cflag) == B0 )
         termios_p->c_ispeed = termios_p->c_ospeed;
  
       if ( (speed & ~CBAUD) != 0 || speed > _MAX_BAUD )
         speed = BOTHER;

       /*
        * Don't set the input flags here; the B0 in c_cflag indicates that
        * the input speed is tied to the output speed.
        */
       termios_p->c_cflag = (termios_p->c_cflag & ~CBAUD) | speed;
       return 0;
     } 
     ---

This allows us to adjust the baud rates to non-standard values using termios
interfaces without to resorting to add new headers and use a different API
(ioctl).

As Peter Anvin has indicated, he create a POC [1] with the aforementioned
new interfaces.  It has not been rebased against master, more specially against
my termios refactor to simplify the multiple architecture header definitions,
but I intend to use as a base.

> 
> 2019-04-09  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #10339]
> 	Linux: Define struct termios2 in <termios.h> under _GNU_SOURCE.
> 	* sysdeps/unix/sysv/linux/Makefile [$(subdir) == termios] (tests):
> 	Add tst-termios2.
> 	* sysdeps/unix/sysv/linux/tst-termios2.c: New file.
> 	* sysdeps/unix/sysv/linux/bits/termios2-struct.h: Likewise.
> 	* sysdeps/unix/sysv/linux/bits/termios.h [__USE_GNU]: Include it.
> 	* sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h: New file.
> 	* sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h: Likewise.
> 
> diff --git a/NEWS b/NEWS
> index b58e2469d4..5e6ecb9c7d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -18,6 +18,9 @@ Major new features:
>  
>  * On Linux, the gettid function has been added.
>  
> +* On Linux, <termios.h> now provides a definition of struct termios2 with
> +  the _GNU_SOURCE feature test macro.
> +
>  * Minguo (Republic of China) calendar support has been added as an
>    alternative calendar for the following locales: zh_TW, cmn_TW, hak_TW,
>    nan_TW, lzh_TW.
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 52ac6ad484..4cb5e4f0d2 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -156,6 +156,7 @@ endif
>  
>  ifeq ($(subdir),termios)
>  sysdep_headers += termio.h
> +tests += tst-termios2
>  endif
>  
>  ifeq ($(subdir),posix)
> diff --git a/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
> new file mode 100644
> index 0000000000..5f09445e23
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/alpha/bits/termios2-struct.h
> @@ -0,0 +1,33 @@
> +/* struct termios2 definition.  Linux/alpha version.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _TERMIOS_H
> +# error "Never include <bits/termios2-struct.h> directly; use <termios.h> instead."
> +#endif
> +
> +struct termios2
> +{
> +  tcflag_t c_iflag;
> +  tcflag_t c_oflag;
> +  tcflag_t c_cflag;
> +  tcflag_t c_lflag;
> +  cc_t c_cc[NCCS];
> +  cc_t c_line;
> +  speed_t c_ispeed;
> +  speed_t c_ospeed;
> +};
> diff --git a/sysdeps/unix/sysv/linux/bits/termios.h b/sysdeps/unix/sysv/linux/bits/termios.h
> index 997231cd03..45ac7affdf 100644
> --- a/sysdeps/unix/sysv/linux/bits/termios.h
> +++ b/sysdeps/unix/sysv/linux/bits/termios.h
> @@ -25,6 +25,10 @@ typedef unsigned int	speed_t;
>  typedef unsigned int	tcflag_t;
>  
>  #include <bits/termios-struct.h>
> +#ifdef __USE_GNU
> +# include <bits/termios2-struct.h>
> +#endif
> +
>  #include <bits/termios-c_cc.h>
>  #include <bits/termios-c_iflag.h>
>  #include <bits/termios-c_oflag.h>
> diff --git a/sysdeps/unix/sysv/linux/bits/termios2-struct.h b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
> new file mode 100644
> index 0000000000..5a48e45ef3
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/termios2-struct.h
> @@ -0,0 +1,33 @@
> +/* struct termios2 definition.  Linux/generic version.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _TERMIOS_H
> +# error "Never include <bits/termios2-struct.h> directly; use <termios.h> instead."
> +#endif
> +
> +struct termios2
> +{
> +  tcflag_t c_iflag;
> +  tcflag_t c_oflag;
> +  tcflag_t c_cflag;
> +  tcflag_t c_lflag;
> +  cc_t c_line;
> +  cc_t c_cc[NCCS];
> +  speed_t c_ispeed;
> +  speed_t c_ospeed;
> +};
> diff --git a/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
> new file mode 100644
> index 0000000000..7c889e575c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/termios2-struct.h
> @@ -0,0 +1,33 @@
> +/* struct termios2 definition.  Linux/sparc version.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _TERMIOS_H
> +# error "Never include <bits/termios2-struct.h> directly; use <termios.h> instead."
> +#endif
> +
> +struct termios2
> +{
> +  tcflag_t c_iflag;
> +  tcflag_t c_oflag;
> +  tcflag_t c_cflag;
> +  tcflag_t c_lflag;
> +  cc_t c_line;
> +  cc_t c_cc[NCCS + 2];
> +  speed_t c_ispeed;
> +  speed_t c_ospeed;
> +};
> diff --git a/sysdeps/unix/sysv/linux/tst-termios2.c b/sysdeps/unix/sysv/linux/tst-termios2.c
> new file mode 100644
> index 0000000000..82326a1288
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-termios2.c
> @@ -0,0 +1,48 @@
> +/* Minimal test of struct termios2 definition.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <termios.h>
> +#include <sys/ioctl.h>
> +
> +/* This function is never executed, but must be compiled successfully.
> +   Accessing serial ports in the test suite is problematic because
> +   they likely correspond with low-level system functionality.  */
> +void
> +not_executed (int fd)
> +{
> +  /* Avoid a compilation failure if TCGETS2, TCSETS2 are not
> +     defined.  */
> +#if defined (TCGETS2) && defined (TCSETS2)
> +  struct termios2 ti;
> +  ioctl (fd, TCGETS2, &ti);
> +  ioctl (fd, TCSETS2, &ti);
> +#endif
> +}
> +
> +static int
> +do_test (void)
> +{
> +  /* Fail at run time if TCGETS2 or TCSETS2 is not defined.  */
> +#if defined (TCGETS2) && defined (TCSETS2)
> +  return 0;
> +#else
> +  return 1;
> +#endif
> +}
> +
> +#include <support/test-driver.c>
>
  

* Adhemerval Zanella:

> This allows us to adjust the baud rates to non-standard values using termios
> interfaces without to resorting to add new headers and use a different API
> (ioctl).

How much symbol versioning will be required for this change?

> As Peter Anvin has indicated, he create a POC [1] with the aforementioned
> new interfaces.  It has not been rebased against master, more specially against
> my termios refactor to simplify the multiple architecture header definitions,
> but I intend to use as a base.

Reference [1] is still missing. 8-(

Thanks,
Florian
  

On 11/04/2019 08:07, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> This allows us to adjust the baud rates to non-standard values using termios
>> interfaces without to resorting to add new headers and use a different API
>> (ioctl).
> 
> How much symbol versioning will be required for this change?

I think all interfaces that have termios as input for sparc and mips 
(tcgetattr, tcsetattr, cfmakeraw, cfgetispeed, cfgetospeed, cfsetispeed,
cfsetospeed, cfsetspeed).

Alpha will also need to use termios1 for pre-4.20 kernels.

> 
>> As Peter Anvin has indicated, he create a POC [1] with the aforementioned
>> new interfaces.  It has not been rebased against master, more specially against
>> my termios refactor to simplify the multiple architecture header definitions,
>> but I intend to use as a base.
> 
> Reference [1] is still missing. 8-(

Oops... it is https://git.zytor.com/users/hpa/glibc/termbaud.git/log/?h=wip.termbaud

> 
> Thanks,
> Florian
>
  

* Adhemerval Zanella:

> On 11/04/2019 08:07, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> This allows us to adjust the baud rates to non-standard values using termios
>>> interfaces without to resorting to add new headers and use a different API
>>> (ioctl).
>> 
>> How much symbol versioning will be required for this change?
>
> I think all interfaces that have termios as input for sparc and mips 
> (tcgetattr, tcsetattr, cfmakeraw, cfgetispeed, cfgetospeed, cfsetispeed,
> cfsetospeed, cfsetspeed).
>
> Alpha will also need to use termios1 for pre-4.20 kernels.

So only new symbol versions there?  Hmm.

>>> As Peter Anvin has indicated, he create a POC [1] with the aforementioned
>>> new interfaces.  It has not been rebased against master, more specially against
>>> my termios refactor to simplify the multiple architecture header definitions,
>>> but I intend to use as a base.
>> 
>> Reference [1] is still missing. 8-(
>
> Oops... it is https://git.zytor.com/users/hpa/glibc/termbaud.git/log/?h=wip.termbaud

This doesn't really illuminate things.  “Drop explicit baud setting
interfaces in favor of cfenc|decspeed()” removes the new symbol version
for the cf* functions.

My gut feeling is that it's safer to add new interfaces, based on the
actual kernel/userspace interface, rather than trying to fix up existing
interfaces with symbol versioning.  The main reason is that code
involving serial interfaces is difficult to test, so it will take years
until we find the last application broken by the glibc interface bump.

I don't feel strongly about this.  This came out of a request for
enabling TCGETS2 support downstream.  If I can't fix this upstream, I
will just reject that request.

Thanks,
Florian
  

* Florian Weimer:

> My gut feeling is that it's safer to add new interfaces, based on the
> actual kernel/userspace interface, rather than trying to fix up existing
> interfaces with symbol versioning.  The main reason is that code
> involving serial interfaces is difficult to test, so it will take years
> until we find the last application broken by the glibc interface bump.
>
> I don't feel strongly about this.  This came out of a request for
> enabling TCGETS2 support downstream.  If I can't fix this upstream, I
> will just reject that request.

Any further comments on this matter?

Thanks,
Florian
  

On April 12, 2019 12:50:41 AM PDT, Florian Weimer <fweimer@redhat.com> wrote:
>* Adhemerval Zanella:
>
>> On 11/04/2019 08:07, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>> 
>>>> This allows us to adjust the baud rates to non-standard values
>using termios
>>>> interfaces without to resorting to add new headers and use a
>different API
>>>> (ioctl).
>>> 
>>> How much symbol versioning will be required for this change?
>>
>> I think all interfaces that have termios as input for sparc and mips 
>> (tcgetattr, tcsetattr, cfmakeraw, cfgetispeed, cfgetospeed,
>cfsetispeed,
>> cfsetospeed, cfsetspeed).
>>
>> Alpha will also need to use termios1 for pre-4.20 kernels.
>
>So only new symbol versions there?  Hmm.
>
>>>> As Peter Anvin has indicated, he create a POC [1] with the
>aforementioned
>>>> new interfaces.  It has not been rebased against master, more
>specially against
>>>> my termios refactor to simplify the multiple architecture header
>definitions,
>>>> but I intend to use as a base.
>>> 
>>> Reference [1] is still missing. 8-(
>>
>> Oops... it is
>https://git.zytor.com/users/hpa/glibc/termbaud.git/log/?h=wip.termbaud
>
>This doesn't really illuminate things.  “Drop explicit baud setting
>interfaces in favor of cfenc|decspeed()” removes the new symbol version
>for the cf* functions.
>
>My gut feeling is that it's safer to add new interfaces, based on the
>actual kernel/userspace interface, rather than trying to fix up
>existing
>interfaces with symbol versioning.  The main reason is that code
>involving serial interfaces is difficult to test, so it will take years
>until we find the last application broken by the glibc interface bump.
>
>I don't feel strongly about this.  This came out of a request for
>enabling TCGETS2 support downstream.  If I can't fix this upstream, I
>will just reject that request.
>
>Thanks,
>Florian

New interfaces are only necessary for the handful of architectures that don't have the speed fields *and* to space to put them in. 

Using symbol versioning doesn't really help much since the real problem is that struct termios can be passed around in userspace, and the interfaces between user space libraries don't have any versioning. However, my POC code deals with that too by only seeing BOTHER when necessary, so if the structure is extended garbage in the extra fields will be ignored unless new baud rates are in use.

My POC code deals with Alpha as well by falling back to the old interfaces if necessary and possible, otherwise return error.

Exporting termios2 to user space feels a bit odd at this stage as it would only be usable as a fallback on old glibc. Call it kernel_termios2 at least. ioctls using struct termios *must* be changed to kernel_termios anyway!
  

On April 9, 2019 11:50:24 PM PDT, Florian Weimer <fweimer@redhat.com> wrote:
>* hpa:
>
>> PowerPC doesn't need it at all.
>
>Because the definition would be the same as struct termios?
>
>I think we should still define struct termios2 using the struct termios
>definition, and also define TCGETS2 and TCSETS2.  This way,
>applications
>can use the struct termios2 type without affecting portability to
>POWER.
>
>Thanks,
>Florian

Agreed. There is just no reason to have a new set of ioctl numbers for PowerPC; just make them aliases.
  

On 15/04/2019 12:53, hpa@zytor.com wrote:
> On April 12, 2019 12:50:41 AM PDT, Florian Weimer <fweimer@redhat.com> wrote:
>> * Adhemerval Zanella:
>>
>>> On 11/04/2019 08:07, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> This allows us to adjust the baud rates to non-standard values
>> using termios
>>>>> interfaces without to resorting to add new headers and use a
>> different API
>>>>> (ioctl).
>>>>
>>>> How much symbol versioning will be required for this change?
>>>
>>> I think all interfaces that have termios as input for sparc and mips 
>>> (tcgetattr, tcsetattr, cfmakeraw, cfgetispeed, cfgetospeed,
>> cfsetispeed,
>>> cfsetospeed, cfsetspeed).
>>>
>>> Alpha will also need to use termios1 for pre-4.20 kernels.
>>
>> So only new symbol versions there?  Hmm.
>>
>>>>> As Peter Anvin has indicated, he create a POC [1] with the
>> aforementioned
>>>>> new interfaces.  It has not been rebased against master, more
>> specially against
>>>>> my termios refactor to simplify the multiple architecture header
>> definitions,
>>>>> but I intend to use as a base.
>>>>
>>>> Reference [1] is still missing. 8-(
>>>
>>> Oops... it is
>> https://git.zytor.com/users/hpa/glibc/termbaud.git/log/?h=wip.termbaud
>>
>> This doesn't really illuminate things.  “Drop explicit baud setting
>> interfaces in favor of cfenc|decspeed()” removes the new symbol version
>> for the cf* functions.
>>
>> My gut feeling is that it's safer to add new interfaces, based on the
>> actual kernel/userspace interface, rather than trying to fix up
>> existing
>> interfaces with symbol versioning.  The main reason is that code
>> involving serial interfaces is difficult to test, so it will take years
>> until we find the last application broken by the glibc interface bump.
>>
>> I don't feel strongly about this.  This came out of a request for
>> enabling TCGETS2 support downstream.  If I can't fix this upstream, I
>> will just reject that request.
>>
>> Thanks,
>> Florian
> 
> New interfaces are only necessary for the handful of architectures that don't have the speed fields *and* to space to put them in. 

Based on your WIP, it seems that both sparc and mips could be adapted.
Do we still have glibc supported architecture that would require compat
symbols?

> 
> Using symbol versioning doesn't really help much since the real problem is that struct termios can be passed around in userspace, and the interfaces between user space libraries don't have any versioning. However, my POC code deals with that too by only seeing BOTHER when necessary, so if the structure is extended garbage in the extra fields will be ignored unless new baud rates are in use.

Yeah, we discussed this earlier and if recall correctly it was not settled
that all architectures would allow the use to extra space for the new
fields. It seems the case, which makes the adaptation for termios2 even easier.

The question I have for kernel side is whether termios2 is fully compatible
with termios, meaning that if there is conner cases we need to handle in
userland.

> 
> My POC code deals with Alpha as well by falling back to the old interfaces if necessary and possible, otherwise return error.
> 
> Exporting termios2 to user space feels a bit odd at this stage as it would only be usable as a fallback on old glibc. Call it kernel_termios2 at least. ioctls using struct termios *must* be changed to kernel_termios anyway!
> 

I still prefer to avoid export it to userland and make it usable through
default termios, as your wip does.  My understanding is new interfaces 
should be semantic equal to current one with the only deviation that
non-standard baudrates will handled as its values.  The only issue I 
can foresee is if POSIX starts to export new bauds value.
  

* hpa:

> Using symbol versioning doesn't really help much since the real
> problem is that struct termios can be passed around in userspace, and
> the interfaces between user space libraries don't have any
> versioning. However, my POC code deals with that too by only seeing
> BOTHER when necessary, so if the structure is extended garbage in the
> extra fields will be ignored unless new baud rates are in use.

That still doesn't solve the problem of changing struct offsets after a
struct field of type struct termios.

> Exporting termios2 to user space feels a bit odd at this stage as it
> would only be usable as a fallback on old glibc. Call it
> kernel_termios2 at least.

I'm not sure why we should do that?  The kernel calls it struct termios2
in its UAPI headers.  If that name is not appropriate, it should be
changed first in the UAPI headers.

Thanks,
Florian
  

On 16/04/2019 06:59, Florian Weimer wrote:
> * hpa:
> 
>> Using symbol versioning doesn't really help much since the real
>> problem is that struct termios can be passed around in userspace, and
>> the interfaces between user space libraries don't have any
>> versioning. However, my POC code deals with that too by only seeing
>> BOTHER when necessary, so if the structure is extended garbage in the
>> extra fields will be ignored unless new baud rates are in use.
> 
> That still doesn't solve the problem of changing struct offsets after a
> struct field of type struct termios.

We will need symbol versioning at least on sparc, since currently it 
defines NCSS 17 and termios-c_cc.h defines 16 control characters (there 
is no space to squeeze more fields one termios).  ANd The WIP branch
gratuitously change the termios struct size on the architecture.  

I am not sure which would be the best option to avoid the the user space 
libraries compatibility issue. It is unlikely to happen, it would require
one to use old libraries along with newer libraries build against a newer
glibc.  Not sure how often this scenarios arises in realworld (specially
on sparc).

I think MIPS would be fine to lower NCSS to 24, as WIP branch does.  And
alpha is also fine since it already provides the c_* fields.

> 
>> Exporting termios2 to user space feels a bit odd at this stage as it
>> would only be usable as a fallback on old glibc. Call it
>> kernel_termios2 at least.
> 
> I'm not sure why we should do that?  The kernel calls it struct termios2
> in its UAPI headers.  If that name is not appropriate, it should be
> changed first in the UAPI headers.
> 
> Thanks,
> Florian
>
  

On Tue, 9 Apr 2019, Florian Weimer wrote:

> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 52ac6ad484..4cb5e4f0d2 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -156,6 +156,7 @@ endif
>  
>  ifeq ($(subdir),termios)
>  sysdep_headers += termio.h
> +tests += tst-termios2
>  endif

This is missing the addition of bits/termios2-struct.h to the installed 
headers.
  

On 4/15/19 10:22 AM, Adhemerval Zanella wrote:
>>
>> New interfaces are only necessary for the handful of architectures that don't have the speed fields *and* to space to put them in. 
> 
> Based on your WIP, it seems that both sparc and mips could be adapted.
> Do we still have glibc supported architecture that would require compat
> symbols?
> 
>>
>> Using symbol versioning doesn't really help much since the real problem is that struct termios can be passed around in userspace, and the interfaces between user space libraries don't have any versioning. However, my POC code deals with that too by only seeing BOTHER when necessary, so if the structure is extended garbage in the extra fields will be ignored unless new baud rates are in use.
> 
> Yeah, we discussed this earlier and if recall correctly it was not settled
> that all architectures would allow the use to extra space for the new
> fields. It seems the case, which makes the adaptation for termios2 even easier.
> 
> The question I have for kernel side is whether termios2 is fully compatible
> with termios, meaning that if there is conner cases we need to handle in
> userland.
> 

It depends on what you mean with "fully compatible."

Functionality-wise, the termios2 interfaces are a strict superset. There
is not, however, any guarantee that struct kernel_termios2 *contains* a
contiguous binary equivalent to struct kernel_termios (in fact, on most
architectures, it doesn't.)

>>
>> My POC code deals with Alpha as well by falling back to the old interfaces if necessary and possible, otherwise return error.
>>
>> Exporting termios2 to user space feels a bit odd at this stage as it would only be usable as a fallback on old glibc. Call it kernel_termios2 at least. ioctls using struct termios *must* be changed to kernel_termios anyway!
>>
> 
> I still prefer to avoid export it to userland and make it usable through
> default termios, as your wip does.  My understanding is new interfaces 
> should be semantic equal to current one with the only deviation that
> non-standard baudrates will handled as its values.  The only issue I 
> can foresee is if POSIX starts to export new bauds value.
> 

... which will be easy to handle: just define a Bxxxx constant with the
value equal to the baud rate.

	-hhpa
  

On 4/16/19 2:59 AM, Florian Weimer wrote:
> * hpa:
> 
>> Using symbol versioning doesn't really help much since the real
>> problem is that struct termios can be passed around in userspace, and
>> the interfaces between user space libraries don't have any
>> versioning. However, my POC code deals with that too by only seeing
>> BOTHER when necessary, so if the structure is extended garbage in the
>> extra fields will be ignored unless new baud rates are in use.
> 
> That still doesn't solve the problem of changing struct offsets after a
> struct field of type struct termios.
> 
>> Exporting termios2 to user space feels a bit odd at this stage as it
>> would only be usable as a fallback on old glibc. Call it
>> kernel_termios2 at least.
> 
> I'm not sure why we should do that?  The kernel calls it struct termios2
> in its UAPI headers.  If that name is not appropriate, it should be
> changed first in the UAPI headers.
> 

It should; I believe the namespace we ought to use is struct
__kernel_termios[2].

"struct termios" defined in these headers is obviously completely wrong
for userspace.

	-hpa
  

On 17/04/2019 19:04, H. Peter Anvin wrote:
> On 4/15/19 10:22 AM, Adhemerval Zanella wrote:
>>>
>>> New interfaces are only necessary for the handful of architectures that don't have the speed fields *and* to space to put them in. 
>>
>> Based on your WIP, it seems that both sparc and mips could be adapted.
>> Do we still have glibc supported architecture that would require compat
>> symbols?
>>
>>>
>>> Using symbol versioning doesn't really help much since the real problem is that struct termios can be passed around in userspace, and the interfaces between user space libraries don't have any versioning. However, my POC code deals with that too by only seeing BOTHER when necessary, so if the structure is extended garbage in the extra fields will be ignored unless new baud rates are in use.
>>
>> Yeah, we discussed this earlier and if recall correctly it was not settled
>> that all architectures would allow the use to extra space for the new
>> fields. It seems the case, which makes the adaptation for termios2 even easier.
>>
>> The question I have for kernel side is whether termios2 is fully compatible
>> with termios, meaning that if there is conner cases we need to handle in
>> userland.
>>
> 
> It depends on what you mean with "fully compatible."
> 
> Functionality-wise, the termios2 interfaces are a strict superset. There
> is not, however, any guarantee that struct kernel_termios2 *contains* a
> contiguous binary equivalent to struct kernel_termios (in fact, on most
> architectures, it doesn't.)

I mean that we can fully implement termios1 using termios2 by adjusting
the termios struct in syscall wrappers.  If it is a superset I think it
is fine.

> 
>>>
>>> My POC code deals with Alpha as well by falling back to the old interfaces if necessary and possible, otherwise return error.
>>>
>>> Exporting termios2 to user space feels a bit odd at this stage as it would only be usable as a fallback on old glibc. Call it kernel_termios2 at least. ioctls using struct termios *must* be changed to kernel_termios anyway!
>>>
>>
>> I still prefer to avoid export it to userland and make it usable through
>> default termios, as your wip does.  My understanding is new interfaces 
>> should be semantic equal to current one with the only deviation that
>> non-standard baudrates will handled as its values.  The only issue I 
>> can foresee is if POSIX starts to export new bauds value.
>>
> 
> ... which will be easy to handle: just define a Bxxxx constant with the
> value equal to the baud rate.
> 
> 	-hhpa

Right.
  

On April 18, 2019 4:09:07 AM PDT, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>
>
>On 17/04/2019 19:04, H. Peter Anvin wrote:
>> On 4/15/19 10:22 AM, Adhemerval Zanella wrote:
>>>>
>>>> New interfaces are only necessary for the handful of architectures
>that don't have the speed fields *and* to space to put them in. 
>>>
>>> Based on your WIP, it seems that both sparc and mips could be
>adapted.
>>> Do we still have glibc supported architecture that would require
>compat
>>> symbols?
>>>
>>>>
>>>> Using symbol versioning doesn't really help much since the real
>problem is that struct termios can be passed around in userspace, and
>the interfaces between user space libraries don't have any versioning.
>However, my POC code deals with that too by only seeing BOTHER when
>necessary, so if the structure is extended garbage in the extra fields
>will be ignored unless new baud rates are in use.
>>>
>>> Yeah, we discussed this earlier and if recall correctly it was not
>settled
>>> that all architectures would allow the use to extra space for the
>new
>>> fields. It seems the case, which makes the adaptation for termios2
>even easier.
>>>
>>> The question I have for kernel side is whether termios2 is fully
>compatible
>>> with termios, meaning that if there is conner cases we need to
>handle in
>>> userland.
>>>
>> 
>> It depends on what you mean with "fully compatible."
>> 
>> Functionality-wise, the termios2 interfaces are a strict superset.
>There
>> is not, however, any guarantee that struct kernel_termios2 *contains*
>a
>> contiguous binary equivalent to struct kernel_termios (in fact, on
>most
>> architectures, it doesn't.)
>
>I mean that we can fully implement termios1 using termios2 by adjusting
>the termios struct in syscall wrappers.  If it is a superset I think it
>is fine.
>
>> 
>>>>
>>>> My POC code deals with Alpha as well by falling back to the old
>interfaces if necessary and possible, otherwise return error.
>>>>
>>>> Exporting termios2 to user space feels a bit odd at this stage as
>it would only be usable as a fallback on old glibc. Call it
>kernel_termios2 at least. ioctls using struct termios *must* be changed
>to kernel_termios anyway!
>>>>
>>>
>>> I still prefer to avoid export it to userland and make it usable
>through
>>> default termios, as your wip does.  My understanding is new
>interfaces 
>>> should be semantic equal to current one with the only deviation that
>>> non-standard baudrates will handled as its values.  The only issue I
>
>>> can foresee is if POSIX starts to export new bauds value.
>>>
>> 
>> ... which will be easy to handle: just define a Bxxxx constant with
>the
>> value equal to the baud rate.
>> 
>> 	-hhpa
>
>Right.

termio, termios1 and termios2 are kernel ioctl interfaces ... there are no wrappers; it is an ioctl.

The glibc termios is different from all of these, and already is a wrapper around the kernel-provided ioctls.