[2/3] Linux: unlockpt needs to fail with EINVAL, not ENOTTY (bug 26053)
Commit Message
The EINVAL error code is mandated by POSIX and documented in the
manual. Also clean up the unlockpt implementation a bit, assuming
that TIOCSPTLCK is always defined.
Enhance login/tst-grantpt to cover unlockpt corner cases.
---
login/tst-grantpt.c | 20 ++++++++++++++++----
sysdeps/unix/sysv/linux/unlockpt.c | 21 +++++----------------
2 files changed, 21 insertions(+), 20 deletions(-)
Comments
On 27/05/2020 07:14, Florian Weimer via Libc-alpha wrote:
> The EINVAL error code is mandated by POSIX and documented in the
> manual. Also clean up the unlockpt implementation a bit, assuming
> that TIOCSPTLCK is always defined.
>
> Enhance login/tst-grantpt to cover unlockpt corner cases.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> login/tst-grantpt.c | 20 ++++++++++++++++----
> sysdeps/unix/sysv/linux/unlockpt.c | 21 +++++----------------
> 2 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/login/tst-grantpt.c b/login/tst-grantpt.c
> index 1d7a220fcf..8ca901ef94 100644
> --- a/login/tst-grantpt.c
> +++ b/login/tst-grantpt.c
> @@ -1,4 +1,4 @@
> -/* Test for grantpt error corner cases.
> +/* Test for grantpt, unlockpt error corner cases.
> Copyright (C) 2001-2020 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
>
> @@ -28,7 +28,7 @@
> #include <support/temp_file.h>
> #include <support/xunistd.h>
>
> -/* Test grantpt with a closed descriptor. */
> +/* Test grantpt, unlockpt with a closed descriptor. */
> static void
> test_ebadf (void)
> {
> @@ -48,9 +48,12 @@ test_ebadf (void)
> printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EBADF);
> printf (" got: return = %d, errno = %d\n", ret, err);
> }
> +
> + TEST_COMPARE (unlockpt (fd), -1);
> + TEST_COMPARE (errno, EBADF);
> }
>
Ok.
> -/* Test grantpt on a regular file. */
> +/* Test grantpt, unlockpt on a regular file. */
> static void
> test_einval (void)
> {
> @@ -68,10 +71,13 @@ test_einval (void)
> printf (" got: return = %d, errno = %d\n", ret, err);
> }
>
> + TEST_COMPARE (unlockpt (fd), -1);
> + TEST_COMPARE (errno, EINVAL);
> +
> xclose (fd);
> }
>
Ok.
> -/* Test grantpt on a non-ptmx pseudo-terminal. */
> +/* Test grantpt, unlockpt on a non-ptmx pseudo-terminal. */
> static void
> test_not_ptmx (void)
> {
> @@ -80,6 +86,9 @@ test_not_ptmx (void)
> TEST_COMPARE (grantpt (ptmx), 0);
> TEST_COMPARE (unlockpt (ptmx), 0);
>
> + /* A second unlock succeeds as well. */
> + TEST_COMPARE (unlockpt (ptmx), 0);
> +
> const char *name = ptsname (ptmx);
> TEST_VERIFY_EXIT (name != NULL);
> int pts = open (name, O_RDWR | O_NOCTTY);
> @@ -88,6 +97,9 @@ test_not_ptmx (void)
> TEST_COMPARE (grantpt (pts), -1);
> TEST_COMPARE (errno, EINVAL);
>
> + TEST_COMPARE (unlockpt (pts), -1);
> + TEST_COMPARE (errno, EINVAL);
> +
> xclose (pts);
> xclose (ptmx);
> }
Ok.
> diff --git a/sysdeps/unix/sysv/linux/unlockpt.c b/sysdeps/unix/sysv/linux/unlockpt.c
> index 3a0ac7a96c..4d98abece0 100644
> --- a/sysdeps/unix/sysv/linux/unlockpt.c
> +++ b/sysdeps/unix/sysv/linux/unlockpt.c
> @@ -27,22 +27,11 @@
> int
> unlockpt (int fd)
> {
> -#ifdef TIOCSPTLCK
> - int save_errno = errno;
> int unlock = 0;
>
> - if (__ioctl (fd, TIOCSPTLCK, &unlock))
> - {
> - if (errno == EINVAL)
> - {
> - __set_errno (save_errno);
> - return 0;
> - }
> - else
> - return -1;
> - }
> -#endif
> - /* If we have no TIOCSPTLCK ioctl, all slave pseudo terminals are
> - unlocked by default. */
> - return 0;
> + int ret = __ioctl (fd, TIOCSPTLCK, &unlock);
> + if (ret != 0 && errno == ENOTTY)
> + /* POSIX mandates EINVAL for non-ptmx descriptors. */
> + __set_errno (EINVAL);
> + return ret;
> }
>
Ok.
@@ -1,4 +1,4 @@
-/* Test for grantpt error corner cases.
+/* Test for grantpt, unlockpt error corner cases.
Copyright (C) 2001-2020 Free Software Foundation, Inc.
This file is part of the GNU C Library.
@@ -28,7 +28,7 @@
#include <support/temp_file.h>
#include <support/xunistd.h>
-/* Test grantpt with a closed descriptor. */
+/* Test grantpt, unlockpt with a closed descriptor. */
static void
test_ebadf (void)
{
@@ -48,9 +48,12 @@ test_ebadf (void)
printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EBADF);
printf (" got: return = %d, errno = %d\n", ret, err);
}
+
+ TEST_COMPARE (unlockpt (fd), -1);
+ TEST_COMPARE (errno, EBADF);
}
-/* Test grantpt on a regular file. */
+/* Test grantpt, unlockpt on a regular file. */
static void
test_einval (void)
{
@@ -68,10 +71,13 @@ test_einval (void)
printf (" got: return = %d, errno = %d\n", ret, err);
}
+ TEST_COMPARE (unlockpt (fd), -1);
+ TEST_COMPARE (errno, EINVAL);
+
xclose (fd);
}
-/* Test grantpt on a non-ptmx pseudo-terminal. */
+/* Test grantpt, unlockpt on a non-ptmx pseudo-terminal. */
static void
test_not_ptmx (void)
{
@@ -80,6 +86,9 @@ test_not_ptmx (void)
TEST_COMPARE (grantpt (ptmx), 0);
TEST_COMPARE (unlockpt (ptmx), 0);
+ /* A second unlock succeeds as well. */
+ TEST_COMPARE (unlockpt (ptmx), 0);
+
const char *name = ptsname (ptmx);
TEST_VERIFY_EXIT (name != NULL);
int pts = open (name, O_RDWR | O_NOCTTY);
@@ -88,6 +97,9 @@ test_not_ptmx (void)
TEST_COMPARE (grantpt (pts), -1);
TEST_COMPARE (errno, EINVAL);
+ TEST_COMPARE (unlockpt (pts), -1);
+ TEST_COMPARE (errno, EINVAL);
+
xclose (pts);
xclose (ptmx);
}
@@ -27,22 +27,11 @@
int
unlockpt (int fd)
{
-#ifdef TIOCSPTLCK
- int save_errno = errno;
int unlock = 0;
- if (__ioctl (fd, TIOCSPTLCK, &unlock))
- {
- if (errno == EINVAL)
- {
- __set_errno (save_errno);
- return 0;
- }
- else
- return -1;
- }
-#endif
- /* If we have no TIOCSPTLCK ioctl, all slave pseudo terminals are
- unlocked by default. */
- return 0;
+ int ret = __ioctl (fd, TIOCSPTLCK, &unlock);
+ if (ret != 0 && errno == ENOTTY)
+ /* POSIX mandates EINVAL for non-ptmx descriptors. */
+ __set_errno (EINVAL);
+ return ret;
}