New functions tcgetwinsize and tcsetwinsize
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
fail
|
Patch failed to apply
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
fail
|
Patch failed to apply
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
fail
|
Patch failed to apply
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
fail
|
Patch failed to apply
|
Commit Message
Good afternoon,
I hope everyone is doing well.
The attached patch is for adding the tcgetwinsize() and tcsetwinsize()
functions that are expected to be a part of POSIX base specifications
issue 8: https://www.austingroupbugs.net/view.php?id=1151#c3856. These
functions get/set tty winsize respectively.
In 2020, I had added these functions to FreeBSD's libc and to the
termios module of cpython. They are also available in NetBSD and in
musl libc. I had prepared a patch for glibc as well, but my university
declined to release rights in 2020. Now I have graduated, and I am
submitting an updated patch.
Notes:
1. If this patch is accepted, then I will write manpages for the functions.
2. This is only implementing the functions on BSD and Linux, and the
implementations are identical: the linux implementation simply
includes the BSD implementation. Should the linux files include
copyright messages as well? Please let me know if I should add it to
other platforms, such as Hurd.
3. I have tested this on Linux: I wrote a custom test (attached file
testwinsize.c) followed by "make check". The custom test is not a part
of the patch. I have not tested it on the BSDs yet. I have also not
tested the stub versions (termios/tcgetwinsize.c,
termios/tcsetwinsize.c); should the stub versions have #include
<stddef.h>, libc_hidden_*, weak_alias, etc in them akin to the stub
version of tcgetattr (termios/tcgetattr.c)?
4. In order to maintain current behavior of <sys/ioctl.h>, I have
added the #include <bits/winsize.h> within #if defined(TIOCGWINSZ) ||
defined(TIOCSWINSZ) guards in <bits/ioctl-types.h>. However, the
#include <bits/winsize.h> in <termios.h> is not within those guards.
5. The #include <bits/winsize.h> along with the tcgetwinsize() and
tcsetwinsize() declarations in <termios.h> are currently within #ifdef
__USE_GNU guards. Please let me know if that should be something else.
When POSIX base specifications issue 8 is released, we will have to
modify those guards and also update glibc/conform/data/termios.h-data.
6. I am still learning about the structure of the glibc source tree,
and wrote this mainly based on grep, find, etc. Are changes to
glibc/include/termios.h and/or glibc/posix/annexc.c necessary? Those
files match when you grep for "tcgetattr".
Thank you for your time.
Sincerely,
Soumendra
Comments
* Soumendra Ganguly:
> In 2020, I had added these functions to FreeBSD's libc and to the
> termios module of cpython. They are also available in NetBSD and in
> musl libc. I had prepared a patch for glibc as well, but my university
> declined to release rights in 2020. Now I have graduated, and I am
> submitting an updated patch.
Does the university have copyright claims on this updated patch?
> 1. If this patch is accepted, then I will write manpages for the
> functions.
Would you be able to add something to the glibc Texinfo manual as well?
At the very least, this needs a NEWS entry.
> 2. This is only implementing the functions on BSD and Linux, and the
> implementations are identical: the linux implementation simply
> includes the BSD implementation. Should the linux files include
> copyright messages as well? Please let me know if I should add it to
> other platforms, such as Hurd.
This should happen automatically if you add it as files to posix/ (or
termios/). Then you don't need a stub version.
If you do not have copyright assignment on file, the copyright notice in
all new and changed files should include the line “Copyright The GNU
Toolchain Authors.”.
Or do you mean preserving the copyright from your BSD contributions?
> 3. I have tested this on Linux: I wrote a custom test (attached file
> testwinsize.c) followed by "make check". The custom test is not a part
> of the patch. I have not tested it on the BSDs yet. I have also not
> tested the stub versions (termios/tcgetwinsize.c,
> termios/tcsetwinsize.c); should the stub versions have #include
> <stddef.h>, libc_hidden_*, weak_alias, etc in them akin to the stub
> version of tcgetattr (termios/tcgetattr.c)?
See above, we can skip this step.
> 4. In order to maintain current behavior of <sys/ioctl.h>, I have
> added the #include <bits/winsize.h> within #if defined(TIOCGWINSZ) ||
> defined(TIOCSWINSZ) guards in <bits/ioctl-types.h>. However, the
> #include <bits/winsize.h> in <termios.h> is not within those guards.
You can use bits/types/struct_winsize.h without guards.
> 5. The #include <bits/winsize.h> along with the tcgetwinsize() and
> tcsetwinsize() declarations in <termios.h> are currently within #ifdef
> __USE_GNU guards. Please let me know if that should be something else.
> When POSIX base specifications issue 8 is released, we will have to
> modify those guards and also update glibc/conform/data/termios.h-data.
You can use __USE_MISC for now, I think we want those functions
available by default because they are also part of BSD.
> 6. I am still learning about the structure of the glibc source tree,
> and wrote this mainly based on grep, find, etc. Are changes to
> glibc/include/termios.h and/or glibc/posix/annexc.c necessary? Those
> files match when you grep for "tcgetattr".
I believe posix/annexc.c does not need to be updated for now.
include/termios.h would need updating once the new functions are called
internally, for symbol management purposes. We can skip this in the
initial patch.
Please make sure that you get clean “make check -j`nproc`” output with
your changes. (You must configure with --prefix=/usr.) Please run
“make update-all-abi” to update all the libc.abilist files with the new
GLIBC_2.39 symbols.
Could you add the test case as termios/tst-winsize.c or something like
that? You'll have to create a new pseudo-terminal, to avoid test
failures when the build process runs out with a terminal.
> diff --git a/termios/Versions b/termios/Versions
> index 711ed0334b..159d4a149e 100644
> --- a/termios/Versions
> +++ b/termios/Versions
> @@ -11,4 +11,8 @@ libc {
> # t*
> tcgetsid;
> }
> + GLIBC_2.38 {
> + # t*
> + tcgetwinsize; tcsetwinsize;
> + }
> }
This should be GLIBC_2.39 now.
Thanks,
Florian
Thank you for the review.
On 8/27/23, Florian Weimer <fweimer@redhat.com> wrote:
> * Soumendra Ganguly:
>
>> In 2020, I had added these functions to FreeBSD's libc and to the
>> termios module of cpython. They are also available in NetBSD and in
>> musl libc. I had prepared a patch for glibc as well, but my university
>> declined to release rights in 2020. Now I have graduated, and I am
>> submitting an updated patch.
>
> Does the university have copyright claims on this updated patch?
>
Fortunately they do not, and I have completed necessary paperwork;
Craig Topham (copyright-clerk@fsf.org) is CC'd.
>> 1. If this patch is accepted, then I will write manpages for the
>> functions.
>
> Would you be able to add something to the glibc Texinfo manual as well?
>
> At the very least, this needs a NEWS entry.
>
Yes, I will.
>> 2. This is only implementing the functions on BSD and Linux, and the
>> implementations are identical: the linux implementation simply
>> includes the BSD implementation. Should the linux files include
>> copyright messages as well? Please let me know if I should add it to
>> other platforms, such as Hurd.
>
> This should happen automatically if you add it as files to posix/ (or
> termios/). Then you don't need a stub version.
>
Is there any OS where ioctl()+TIOCGWINSZ/TIOCSWINSZ is not currently
available? If **yes**, then unfortunately those OSes will require the
stub. The only other approach that I know is via TIOCGSIZE/TIOCSSIZE,
which are VERY old macros, and I am not sure if any OS is still using
them. If **no**, then I might add them to posix/ (or maybe even
termios/) instead.
> If you do not have copyright assignment on file, the copyright notice in
> all new and changed files should include the line “Copyright The GNU
> Toolchain Authors.”.
>
> Or do you mean preserving the copyright from your BSD contributions?
>
No, I was not referring to my BSD contributions. I just wanted to make
sure if I need to add the standard copyright notice even though the
contents of the file is comprised of a single #include. I will add
them :)
>> 3. I have tested this on Linux: I wrote a custom test (attached file
>> testwinsize.c) followed by "make check". The custom test is not a part
>> of the patch. I have not tested it on the BSDs yet. I have also not
>> tested the stub versions (termios/tcgetwinsize.c,
>> termios/tcsetwinsize.c); should the stub versions have #include
>> <stddef.h>, libc_hidden_*, weak_alias, etc in them akin to the stub
>> version of tcgetattr (termios/tcgetattr.c)?
>
> See above, we can skip this step.
>
This will depend upon the answer to the preceding new question.
>> 4. In order to maintain current behavior of <sys/ioctl.h>, I have
>> added the #include <bits/winsize.h> within #if defined(TIOCGWINSZ) ||
>> defined(TIOCSWINSZ) guards in <bits/ioctl-types.h>. However, the
>> #include <bits/winsize.h> in <termios.h> is not within those guards.
>
> You can use bits/types/struct_winsize.h without guards.
>
Perfect! Was unaware of this.
>> 5. The #include <bits/winsize.h> along with the tcgetwinsize() and
>> tcsetwinsize() declarations in <termios.h> are currently within #ifdef
>> __USE_GNU guards. Please let me know if that should be something else.
>> When POSIX base specifications issue 8 is released, we will have to
>> modify those guards and also update glibc/conform/data/termios.h-data.
>
> You can use __USE_MISC for now, I think we want those functions
> available by default because they are also part of BSD.
>
Good. I will also update termios.h-data if necessary.
>> 6. I am still learning about the structure of the glibc source tree,
>> and wrote this mainly based on grep, find, etc. Are changes to
>> glibc/include/termios.h and/or glibc/posix/annexc.c necessary? Those
>> files match when you grep for "tcgetattr".
>
> I believe posix/annexc.c does not need to be updated for now.
>
> include/termios.h would need updating once the new functions are called
> internally, for symbol management purposes. We can skip this in the
> initial patch.
>
Makes sense.
> Please make sure that you get clean “make check -j`nproc`” output with
> your changes. (You must configure with --prefix=/usr.) Please run
> “make update-all-abi” to update all the libc.abilist files with the new
> GLIBC_2.39 symbols.
>
Will try this next.
> Could you add the test case as termios/tst-winsize.c or something like
> that? You'll have to create a new pseudo-terminal, to avoid test
> failures when the build process runs out with a terminal.
>
Yes, I can do this.
>> diff --git a/termios/Versions b/termios/Versions
>> index 711ed0334b..159d4a149e 100644
>> --- a/termios/Versions
>> +++ b/termios/Versions
>> @@ -11,4 +11,8 @@ libc {
>> # t*
>> tcgetsid;
>> }
>> + GLIBC_2.38 {
>> + # t*
>> + tcgetwinsize; tcsetwinsize;
>> + }
>> }
>
> This should be GLIBC_2.39 now.
>
Got it!
Please give me a few days to work on this. I have some traveling
coming up very soon.
> Thanks,
> Florian
>
>
Sincerely,
Soumendra
From 84bb81d6ff8e441dfd45825742d557ae61065b46 Mon Sep 17 00:00:00 2001
From: Soumendra Ganguly <soumendraganguly@gmail.com>
Date: Sun, 27 Aug 2023 17:56:34 -0500
Subject: [PATCH] Add functions tcgetwinsize() and tcsetwinsize() which are set
to appear in the upcoming POSIX Base Specifications, Issue 8.
---
bits/ioctl-types.h | 13 ++-----
bits/winsize.h | 36 +++++++++++++++++
sysdeps/unix/bsd/tcgetwinsize.c | 25 ++++++++++++
sysdeps/unix/bsd/tcsetwinsize.c | 25 ++++++++++++
sysdeps/unix/sysv/linux/bits/ioctl-types.h | 11 ++----
.../unix/sysv/linux/mips/bits/ioctl-types.h | 10 ++---
.../sysv/linux/powerpc/bits/ioctl-types.h | 11 ++----
sysdeps/unix/sysv/linux/tcgetwinsize.c | 1 +
sysdeps/unix/sysv/linux/tcsetwinsize.c | 1 +
termios/Makefile | 3 +-
termios/Versions | 4 ++
termios/tcgetwinsize.c | 39 +++++++++++++++++++
termios/tcsetwinsize.c | 39 +++++++++++++++++++
termios/termios.h | 11 ++++++
14 files changed, 195 insertions(+), 34 deletions(-)
create mode 100644 bits/winsize.h
create mode 100644 sysdeps/unix/bsd/tcgetwinsize.c
create mode 100644 sysdeps/unix/bsd/tcsetwinsize.c
create mode 100644 sysdeps/unix/sysv/linux/tcgetwinsize.c
create mode 100644 sysdeps/unix/sysv/linux/tcsetwinsize.c
create mode 100644 termios/tcgetwinsize.c
create mode 100644 termios/tcsetwinsize.c
@@ -66,16 +66,9 @@ struct sgttyb
_IOT (_IOTS (char), 4, _IOTS (short int), 1, 0, 0)
#if defined TIOCGWINSZ || defined TIOCSWINSZ
-/* Type of ARG for TIOCGWINSZ and TIOCSWINSZ requests. */
-struct winsize
-{
- unsigned short int ws_row; /* Rows, in characters. */
- unsigned short int ws_col; /* Columns, in characters. */
-
- /* These are not actually used. */
- unsigned short int ws_xpixel; /* Horizontal pixels. */
- unsigned short int ws_ypixel; /* Vertical pixels. */
-};
+/* Get the definition of 'struct winsize', which is the type of ARG for
+ TIOCGWINSZ and TIOCSWINSZ requests. */
+#include <bits/winsize.h>
# define _IOT_winsize /* Hurd ioctl type field. */ \
_IOT (_IOTS (unsigned short int), 4, 0, 0, 0, 0)
new file mode 100644
@@ -0,0 +1,36 @@
+/* Copyright (C) 2023 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
+ <https://www.gnu.org/licenses/>. */
+
+#if !defined(_SYS_IOCTL_H) && !defined(_TERMIOS_H)
+# error "Never use <bits/winsize.h> directly; include <sys/ioctl.h> or <termios.h> instead."
+#endif
+
+#ifndef _BITS_WINSIZE_H
+#define _BITS_WINSIZE_H
+
+/* Type of ARG for TIOCGWINSZ and TIOCSWINSZ requests. */
+struct winsize
+{
+ unsigned short int ws_row; /* Rows, in characters. */
+ unsigned short int ws_col; /* Columns, in characters. */
+
+ /* These are not actually used. */
+ unsigned short int ws_xpixel; /* Horizontal pixels. */
+ unsigned short int ws_ypixel; /* Vertical pixels. */
+};
+
+#endif /* _BITS_WINSIZE_H */
new file mode 100644
@@ -0,0 +1,25 @@
+/* Copyright (C) 2023 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
+ <https://www.gnu.org/licenses/>. */
+
+#include <sys/ioctl.h>
+
+/* Get the winsize of FD in *WINSIZE_P. */
+int
+tcgetwinsize (int fd, struct winsize *winsize_p)
+{
+ return __ioctl (fd, TIOCGWINSZ, winsize_p);
+}
new file mode 100644
@@ -0,0 +1,25 @@
+/* Copyright (C) 2023 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
+ <https://www.gnu.org/licenses/>. */
+
+#include <sys/ioctl.h>
+
+/* Set the winsize of FD to *WINSIZE_P. */
+int
+tcsetwinsize (int fd, const struct winsize *winsize_p)
+{
+ return __ioctl (fd, TIOCSWINSZ, winsize_p);
+}
@@ -23,14 +23,9 @@
/* Get definition of constants for use with `ioctl'. */
#include <asm/ioctls.h>
-
-struct winsize
- {
- unsigned short int ws_row;
- unsigned short int ws_col;
- unsigned short int ws_xpixel;
- unsigned short int ws_ypixel;
- };
+/* Get the definition of 'struct winsize', which is the type of ARG for
+ TIOCGWINSZ and TIOCSWINSZ requests. */
+#include <bits/winsize.h>
#define NCC 8
struct termio
@@ -23,13 +23,9 @@
/* Get definition of constants for use with `ioctl'. */
#include <asm/ioctls.h>
-struct winsize
- {
- unsigned short int ws_row;
- unsigned short int ws_col;
- unsigned short int ws_xpixel;
- unsigned short int ws_ypixel;
- };
+/* Get the definition of 'struct winsize', which is the type of ARG for
+ TIOCGWINSZ and TIOCSWINSZ requests. */
+#include <bits/winsize.h>
#define NCC 8
struct termio
@@ -23,14 +23,9 @@
/* Get definition of constants for use with `ioctl'. */
#include <asm/ioctls.h>
-
-struct winsize
- {
- unsigned short int ws_row;
- unsigned short int ws_col;
- unsigned short int ws_xpixel;
- unsigned short int ws_ypixel;
- };
+/* Get the definition of 'struct winsize', which is the type of ARG for
+ TIOCGWINSZ and TIOCSWINSZ requests. */
+#include <bits/winsize.h>
#define NCC 10
struct termio
new file mode 100644
@@ -0,0 +1 @@
+#include <sysdeps/unix/bsd/tcgetwinsize.c>
new file mode 100644
@@ -0,0 +1 @@
+#include <sysdeps/unix/bsd/tcsetwinsize.c>
@@ -26,7 +26,8 @@ headers := termios.h bits/termios.h sys/ttydefaults.h sys/termios.h \
sys/ttychars.h
routines := speed cfsetspeed tcsetattr tcgetattr tcgetpgrp tcsetpgrp \
- tcdrain tcflow tcflush tcsendbrk cfmakeraw tcgetsid
+ tcdrain tcflow tcflush tcsendbrk cfmakeraw tcgetsid \
+ tcgetwinsize tcsetwinsize
include ../Rules
@@ -11,4 +11,8 @@ libc {
# t*
tcgetsid;
}
+ GLIBC_2.38 {
+ # t*
+ tcgetwinsize; tcsetwinsize;
+ }
}
new file mode 100644
@@ -0,0 +1,39 @@
+/* Copyright (C) 2023 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
+ <https://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <termios.h>
+
+/* Get the winsize of FD in *WINSIZE_P. */
+int
+tcgetwinsize (int fd, struct winsize *winsize_p)
+{
+ if (fd < 0)
+ {
+ __set_errno (EBADF);
+ return -1;
+ }
+ if (winsize_p == NULL)
+ {
+ __set_errno (EINVAL);
+ return -1;
+ }
+
+ __set_errno (ENOSYS);
+ return -1;
+}
+stub_warning (tcgetwinsize)
new file mode 100644
@@ -0,0 +1,39 @@
+/* Copyright (C) 2023 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
+ <https://www.gnu.org/licenses/>. */
+
+#include <errno.h>
+#include <termios.h>
+
+/* Set the winsize of FD to *WINSIZE_P. */
+int
+tcsetwinsize (int fd, const struct winsize *winsize_p)
+{
+ if (fd < 0)
+ {
+ __set_errno (EBADF);
+ return -1;
+ }
+ if (winsize_p == NULL)
+ {
+ __set_errno (EINVAL);
+ return -1;
+ }
+
+ __set_errno (ENOSYS);
+ return -1;
+}
+stub_warning (tcsetwinsize)
@@ -99,6 +99,17 @@ extern int tcflow (int __fd, int __action) __THROW;
extern __pid_t tcgetsid (int __fd) __THROW;
#endif
+#ifdef __USE_GNU
+/* Get the definition of 'struct winsize', which is the type of ARG for
+ TIOCGWINSZ and TIOCSWINSZ requests. */
+#include <bits/winsize.h>
+
+/* Get the winsize of FD in *WINSIZE_P. */
+extern int tcgetwinsize (int __fd, struct winsize *__winsize_p) __THROW;
+
+/* Set the winsize of FD to *WINSIZE_P. */
+extern int tcsetwinsize (int __fd, const struct winsize *__winsize_p) __THROW;
+#endif
#ifdef __USE_MISC
# include <sys/ttydefaults.h>
--
2.30.2