Re: [patch] lib/login_tty.c: Controlling terminal in OpenBSD.

[Bruno Haible]

Hello Mats,

Thank you for the report.

> Without my changes the telnet session does not get a
> controlling terminal

It shouldn't need 'telnet' to detect this. The unit test should find out
about it already. I'm adding the first patch below.

With this unit test in place, indeed I get a test failure on OpenBSD 4.5.

> diff --git a/lib/login_tty.c b/lib/login_tty.c
> index 0403391..e06e916 100644
> --- a/lib/login_tty.c
> +++ b/lib/login_tty.c
> @@ -22,6 +22,10 @@
>  #include <fcntl.h>
>  #include <unistd.h>
>  
> +#if __OpenBSD__
> +# include <sys/ioctl.h>
> +#endif
> +
>  int
>  login_tty (int slave_fd)
>  {
> @@ -53,6 +57,11 @@ login_tty (int slave_fd)
>      close (dummy_fd);
>    }
>  
> +#if __OpenBSD__
> +  /* Needed to activate the controlling terminal, see tty(4).  */
> +  ioctl (slave_fd, TIOCSCTTY, NULL);
> +#endif
> +
>    /* Assign fd to the standard input, standard output, and standardd error of
>       the current process.  */
>    for (i = 0; i < 3; i++)
> 

Thanks for the hint. Other platforms have TIOCSCTTY as well. And, of course,
the return value of every system call must be checked. I'm applying the second
patch below.

But actually, the root of the problem is that the autoconf check has found
"checking for login_tty... no" when OpenBSD _does_ have login_tty. The third
patch below fixes this autoconf check to report "checking for login_tty... yes".

Bruno


2010-09-16  Bruno Haible  <address@hidden>

        login_tty: Stricter unit test.
        * modules/login_tty-tests (Depends-on): Add tcgetsid.
        * tests/test-login_tty.c (main): Also check the results of tcgetpgrp()
        and tcgetsid() after login_tty.
        Reported by Mats Erik Andersson <address@hidden>.

--- modules/login_tty-tests.orig        Fri Sep 17 03:55:06 2010
+++ modules/login_tty-tests     Fri Sep 17 03:19:47 2010
@@ -3,6 +3,7 @@
 
 Depends-on:
 openpty
+tcgetsid
 
 configure.ac:
 
--- tests/test-login_tty.c.orig Fri Sep 17 03:55:06 2010
+++ tests/test-login_tty.c      Fri Sep 17 03:43:23 2010
@@ -19,8 +19,12 @@
 /* Specification.  */
 extern int login_tty (int);
 
+#include <errno.h>
 #include <pty.h>
 #include <stdio.h>
+#include <stdlib.h>
+#include <termios.h>
+#include <unistd.h>
 
 int
 main ()
@@ -48,5 +52,23 @@
       }
   }
 
+  /* From here on, we cannot use stderr for error messages any more.
+     If a test fails, just abort.  */
+
+  /* Check that fd = 0, 1, 2 are now open to the controlling terminal for the
+     current process and that it is a session of its own.  */
+  {
+    int fd;
+    for (fd = 0; fd < 3; fd++)
+      if (!(tcgetpgrp (fd) == getpid ()))
+        abort ();
+    for (fd = 0; fd < 3; fd++)
+      {
+        int sid = tcgetsid (fd);
+        if (!(sid == -1 ? errno == ENOSYS : sid == getpid ()))
+          abort ();
+      }
+  }
+
   return 0;
 }


2010-09-16  Bruno Haible  <address@hidden>

        login_tty: Make the replacement code work on BSD systems.
        * lib/login_tty.c: Include <sys/ioctl.h>.
        (login_tty): Use ioctl TIOCSCTTY when available.
        * modules/login_tty (Depends-on): Add sys_ioctl.
        Reported by Mats Erik Andersson <address@hidden>.

--- lib/login_tty.c.orig        Fri Sep 17 04:03:39 2010
+++ lib/login_tty.c     Fri Sep 17 03:28:57 2010
@@ -21,6 +21,7 @@
 
 #include <fcntl.h>
 #include <unistd.h>
+#include <sys/ioctl.h>
 
 int
 login_tty (int slave_fd)
@@ -31,15 +32,22 @@
   setsid ();
 
   /* Make fd the controlling terminal for the current process.
-     On Solaris: A terminal becomes the controlling terminal of a session
-     if it is being open()ed, at a moment when
-       1. it is not already the controlling terminal of some session, and
-       2. the process that open()s it is a session leader that does not have
-          a controlling terminal.
-     We assume condition 1, try to ensure condition 2, and then open() it.  */
+     On BSD and OSF/1: There is ioctl TIOCSCTTY for this purpose.
+     On Solaris:
+       A terminal becomes the controlling terminal of a session
+       if it is being open()ed, at a moment when
+         1. it is not already the controlling terminal of some session, and
+         2. the process that open()s it is a session leader that does not have
+            a controlling terminal.
+       We assume condition 1, try to ensure condition 2, and then open() it.
+   */
   for (i = 0; i < 3; i++)
     if (i != slave_fd)
       close (i);
+#ifdef TIOCSCTTY
+  if (ioctl (slave_fd, TIOCSCTTY, NULL) < 0)
+    return -1;
+#else
   {
     char *slave_name;
     int dummy_fd;
@@ -52,6 +60,7 @@
       return -1;
     close (dummy_fd);
   }
+#endif
 
   /* Assign fd to the standard input, standard output, and standard error of
      the current process.  */
--- modules/login_tty.orig      Fri Sep 17 04:03:39 2010
+++ modules/login_tty   Fri Sep 17 03:23:05 2010
@@ -8,6 +8,7 @@
 
 Depends-on:
 pty
+sys_ioctl
 
 configure.ac:
 gl_FUNC_LOGIN_TTY


2010-09-16  Bruno Haible  <address@hidden>

        login_tty: Fix detection of function on FreeBSD, OpenBSD, NetBSD.
        * m4/pty.m4 (gl_FUNC_LOGIN_TTY): Augment LIBS while checking whether
        login_tty exists.
        Reported by Mats Erik Andersson <address@hidden>.

--- m4/pty.m4.orig      Fri Sep 17 04:08:33 2010
+++ m4/pty.m4   Fri Sep 17 03:47:53 2010
@@ -1,4 +1,4 @@
-# pty.m4 serial 8
+# pty.m4 serial 9
 dnl Copyright (C) 2010 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -142,7 +142,10 @@
 [
   AC_REQUIRE([gl_PTY_LIB])
 
-  AC_CHECK_FUNCS_ONCE([login_tty])
+  gl_saved_libs="$LIBS"
+  LIBS="$LIBS $PTY_LIB"
+  AC_CHECK_FUNCS([login_tty])
+  LIBS="$gl_saved_LIBS"
   if test $ac_cv_func_login_tty = no; then
     AC_LIBOBJ([login_tty])
   fi