diff mbox series

[1/3] login/tst-grantpt: Convert to support framework, more error checking

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

Commit Message

Florian Weimer Aug. 5, 2020, 7:14 a.m. UTC
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.
---
 login/tst-grantpt.c | 93 ++++++++++++++++++++++++++++-----------------
 1 file changed, 58 insertions(+), 35 deletions(-)

Comments

Adhemerval Zanella Aug. 13, 2020, 6:21 p.m. UTC | #1
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.
Florian Weimer Aug. 13, 2020, 6:31 p.m. UTC | #2
* 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.
Adhemerval Zanella Aug. 13, 2020, 6:58 p.m. UTC | #3
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 mbox series

Patch

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>