Message ID | 0c9120dce321f606d15090f8832039b122b056a0.1596611597.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | [1/3] login/tst-grantpt: Convert to support framework, more error checking | expand |
On 05/08/2020 04:14, Florian Weimer via Libc-alpha wrote: > The test now requires working /dev/pts pseudo-terminals. > > A new subtest (test_not_ptmx) attempts to call grantpt on a > pseudo-terminal that is not a ptmx device. POSIX requires an EINVAL > error in this case. LGTM thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > login/tst-grantpt.c | 93 ++++++++++++++++++++++++++++----------------- > 1 file changed, 58 insertions(+), 35 deletions(-) > > diff --git a/login/tst-grantpt.c b/login/tst-grantpt.c > index 65bb344909..1d7a220fcf 100644 > --- a/login/tst-grantpt.c > +++ b/login/tst-grantpt.c > @@ -1,3 +1,21 @@ > +/* Test for grantpt error corner cases. > + Copyright (C) 2001-2020 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/types.h> > #include <sys/stat.h> > #include <fcntl.h> > @@ -6,76 +24,81 @@ > #include <errno.h> > #include <string.h> > #include <unistd.h> > +#include <support/check.h> > +#include <support/temp_file.h> > +#include <support/xunistd.h> > > -static int > +/* Test grantpt with a closed descriptor. */ > +static void > test_ebadf (void) > { > int fd, ret, err; > > fd = posix_openpt (O_RDWR); > if (fd == -1) > - { > - printf ("posix_openpt(O_RDWR) failed\nerrno %d (%s)\n", > - errno, strerror (errno)); > - /* We don't fail because of this; maybe the system does not have > - SUS pseudo terminals. */ > - return 0; > - } > - unlockpt (fd); > - close (fd); > + FAIL_EXIT1 ("posix_openpt(O_RDWR) failed\nerrno %d (%m)\n", errno); > + TEST_COMPARE (unlockpt (fd), 0); > Ok. > + xclose (fd); > ret = grantpt (fd); > err = errno; > if (ret != -1 || err != EBADF) > { > + support_record_failure (); > printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EBADF); > printf (" got: return = %d, errno = %d\n", ret, err); > - return 1; > } > - return 0; > } Ok (although maybe use CHECK_VERIFY?). > > -static int > +/* Test grantpt on a regular file. */ > +static void > test_einval (void) > { > int fd, ret, err; > - const char file[] = "./grantpt-einval"; > > - fd = open (file, O_RDWR | O_CREAT, 0600); > - if (fd == -1) > - { > - printf ("open(\"%s\", O_RDWR) failed\nerrno %d (%s)\n", > - file, errno, strerror (errno)); > - return 0; > - } > - unlink (file); > + fd = create_temp_file ("tst-grantpt-", NULL); > + TEST_VERIFY_EXIT (fd >= 0); > Ok. As a side note, maybe we should add a xcreate_temp_file. > ret = grantpt (fd); > err = errno; > if (ret != -1 || err != EINVAL) > { > + support_record_failure (); > printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EINVAL); > printf (" got: return = %d, errno = %d\n", ret, err); > - ret = 1; > } > - else > - ret = 0; > > - close (fd); > + xclose (fd); > +} Ok (although same previous suggestion applies here suggestion). > + > +/* Test grantpt on a non-ptmx pseudo-terminal. */ > +static void > +test_not_ptmx (void) > +{ > + int ptmx = posix_openpt (O_RDWR); > + TEST_VERIFY_EXIT (ptmx >= 0); > + TEST_COMPARE (grantpt (ptmx), 0); > + TEST_COMPARE (unlockpt (ptmx), 0); > + > + const char *name = ptsname (ptmx); > + TEST_VERIFY_EXIT (name != NULL); > + int pts = open (name, O_RDWR | O_NOCTTY); > + TEST_VERIFY_EXIT (pts >= 0); > + > + TEST_COMPARE (grantpt (pts), -1); > + TEST_COMPARE (errno, EINVAL); > > - return ret; > + xclose (pts); > + xclose (ptmx); > } > Ok. > static int > do_test (void) > { > - int result = 0; > - > - result += test_ebadf (); > - result += test_einval (); > - > - return result; > + test_ebadf (); > + test_einval (); > + test_not_ptmx (); > + return 0; > } > > -#define TEST_FUNCTION do_test () > -#include "../test-skeleton.c" > +#include <support/test-driver.c> > Ok.
* Adhemerval Zanella via Libc-alpha: > > + xclose (fd); > > ret = grantpt (fd); > > err = errno; > > if (ret != -1 || err != EBADF) > > { > > + support_record_failure (); > > printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EBADF); > > printf (" got: return = %d, errno = %d\n", ret, err); > > - return 1; > > } > > - return 0; > > } > > Ok (although maybe use CHECK_VERIFY?). Do you mean TEST_VERIFY? That can't produce a log message. We probably should have something that logs a failure unconditionally. >> - unlink (file); >> + fd = create_temp_file ("tst-grantpt-", NULL); >> + TEST_VERIFY_EXIT (fd >= 0); >> > > Ok. As a side note, maybe we should add a xcreate_temp_file. I think we should change the semantics of the existing function instead.
On 13/08/2020 15:31, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >>> + xclose (fd); >>> ret = grantpt (fd); >>> err = errno; >>> if (ret != -1 || err != EBADF) >>> { >>> + support_record_failure (); >>> printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EBADF); >>> printf (" got: return = %d, errno = %d\n", ret, err); >>> - return 1; >>> } >>> - return 0; >>> } >> >> Ok (although maybe use CHECK_VERIFY?). > > Do you mean TEST_VERIFY? That can't produce a log message. We > probably should have something that logs a failure unconditionally. > Yeap, TEST_VERIFY. The test failure output will have all the required information anyway (either if return code or errno does not match expectations). >>> - unlink (file); >>> + fd = create_temp_file ("tst-grantpt-", NULL); >>> + TEST_VERIFY_EXIT (fd >= 0); >>> >> >> Ok. As a side note, maybe we should add a xcreate_temp_file. > > I think we should change the semantics of the existing function > instead. > The posix/tst-spawn3.c uses create_temp_file to fill all possible file-descriptor to check if posix_spawn file actions does not use extra descriptors. Afaik it is a exception, all other tests expect that create_temp_file does not fail.
diff --git a/login/tst-grantpt.c b/login/tst-grantpt.c index 65bb344909..1d7a220fcf 100644 --- a/login/tst-grantpt.c +++ b/login/tst-grantpt.c @@ -1,3 +1,21 @@ +/* Test for grantpt error corner cases. + Copyright (C) 2001-2020 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/types.h> #include <sys/stat.h> #include <fcntl.h> @@ -6,76 +24,81 @@ #include <errno.h> #include <string.h> #include <unistd.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <support/xunistd.h> -static int +/* Test grantpt with a closed descriptor. */ +static void test_ebadf (void) { int fd, ret, err; fd = posix_openpt (O_RDWR); if (fd == -1) - { - printf ("posix_openpt(O_RDWR) failed\nerrno %d (%s)\n", - errno, strerror (errno)); - /* We don't fail because of this; maybe the system does not have - SUS pseudo terminals. */ - return 0; - } - unlockpt (fd); - close (fd); + FAIL_EXIT1 ("posix_openpt(O_RDWR) failed\nerrno %d (%m)\n", errno); + TEST_COMPARE (unlockpt (fd), 0); + xclose (fd); ret = grantpt (fd); err = errno; if (ret != -1 || err != EBADF) { + support_record_failure (); printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EBADF); printf (" got: return = %d, errno = %d\n", ret, err); - return 1; } - return 0; } -static int +/* Test grantpt on a regular file. */ +static void test_einval (void) { int fd, ret, err; - const char file[] = "./grantpt-einval"; - fd = open (file, O_RDWR | O_CREAT, 0600); - if (fd == -1) - { - printf ("open(\"%s\", O_RDWR) failed\nerrno %d (%s)\n", - file, errno, strerror (errno)); - return 0; - } - unlink (file); + fd = create_temp_file ("tst-grantpt-", NULL); + TEST_VERIFY_EXIT (fd >= 0); ret = grantpt (fd); err = errno; if (ret != -1 || err != EINVAL) { + support_record_failure (); printf ("grantpt(): expected: return = %d, errno = %d\n", -1, EINVAL); printf (" got: return = %d, errno = %d\n", ret, err); - ret = 1; } - else - ret = 0; - close (fd); + xclose (fd); +} + +/* Test grantpt on a non-ptmx pseudo-terminal. */ +static void +test_not_ptmx (void) +{ + int ptmx = posix_openpt (O_RDWR); + TEST_VERIFY_EXIT (ptmx >= 0); + TEST_COMPARE (grantpt (ptmx), 0); + TEST_COMPARE (unlockpt (ptmx), 0); + + const char *name = ptsname (ptmx); + TEST_VERIFY_EXIT (name != NULL); + int pts = open (name, O_RDWR | O_NOCTTY); + TEST_VERIFY_EXIT (pts >= 0); + + TEST_COMPARE (grantpt (pts), -1); + TEST_COMPARE (errno, EINVAL); - return ret; + xclose (pts); + xclose (ptmx); } static int do_test (void) { - int result = 0; - - result += test_ebadf (); - result += test_einval (); - - return result; + test_ebadf (); + test_einval (); + test_not_ptmx (); + return 0; } -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c" +#include <support/test-driver.c>