[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
Related show

Commit Message

Vitaly Buka via Libc-alpha Aug. 5, 2020, 7:14 a.m.
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(-)

-- 
2.26.2

Comments

Vitaly Buka via Libc-alpha Aug. 13, 2020, 6:21 p.m. | #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. | #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.
Vitaly Buka via Libc-alpha Aug. 13, 2020, 6:58 p.m. | #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.

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>