pt_chown [was: license relaxation request]

[Eric Blake]

On 10/19/2011 03:25 PM, Eric Blake wrote:
> Also, I got a build failure when trying to use the grantpt module in
> libvirt:
>
> CCLD libgnu.la
> CC pt_chown.o
> make[4]: *** No rule to make target `libgnu.a', needed by `pt_chown'. Stop.
>
> Looks like the gnulib-tool output is not considering the possibility of
> libtool mixing with the creation of pt_chown.

Changing pt_chown to not require libgnu.a in any form would be one 
solution; doable by altering the command signature (yes, this would 
differ from glibc's signature, where pt_chown was originally borrowed, 
but since you are installing it into pkglibexecdir, only $PACKAGE should 
be calling it, so it doesn't matter if the calling convention differs). 
 The second patch below implements this idea.

> For that matter, does
> pt_chown even need to be built on Linux, or can we rework things to only
> build it on platforms where the grantpt() replacement is compiled?

Does it even make sense to have pt_chown as a separate module?  It seems 
like the only time you would ever want to build this helper app is in 
the context of grantpt(), so why not inline it into the grantpt module? 
 Besides, it wasn't even mentioned in MODULES.html.  The first patch 
below would solve this.

Are these patches okay?  I've tested them with libvirt, where my 
compilation problem with libtool was resolved.


From a7c290b42d9f56cf8deac6f1875b89419aac0344 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 19 Oct 2011 16:32:32 -0600
Subject: [PATCH 1/2] grantpt: only build pt_chown when needed

Any platform (like Linux) that already has a working grantpt()
does not need the headache of a pt_chown helper app.  There's
no point in offering pt_chown as a separate module, since the
application has to be installed setuid to be useful, and even
then, it is only useful to grantpt().

* modules/pt_chown: Delete, merging into...
* modules/grantpt: ...its sole user.  Make building of pt_chown
depend on an automake conditional.
* NEWS: Document the change.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog        |    6 ++++++
 NEWS             |    5 +++++
 modules/grantpt  |   10 +++++++++-
 modules/pt_chown |   26 --------------------------
 4 files changed, 20 insertions(+), 27 deletions(-)
 delete mode 100644 modules/pt_chown

diff --git a/ChangeLog b/ChangeLog
index a19154a..49524c2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2011-10-19  Eric Blake  <address@hidden>

+       grantpt: only build pt_chown when needed
+       * modules/pt_chown: Delete, merging into...
+       * modules/grantpt: ...its sole user.  Make building of pt_chown
+       depend on an automake conditional.
+       * NEWS: Document the change.
+
        pt_chown: use configmake to simplify build
        * modules/pt_chown (Makefile.am): Drop line guaranteed by configmake.

diff --git a/NEWS b/NEWS
index 2e98e61..493a25b 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,11 @@ User visible incompatible changes

 Date        Modules         Changes

+2011-10-19  pt_chown        This module no longer exists.  The helper
+                            application pt_chown is now only built if 
needed
+                            for the 'grantpt' module, since it has no use
+                            as a stand-alone application.
+
 2011-10-03  poll            The link requirements of this module are 
changed
                             from empty to $(LIB_POLL).

diff --git a/modules/grantpt b/modules/grantpt
index 5fd6537..f317faf 100644
--- a/modules/grantpt
+++ b/modules/grantpt
@@ -3,14 +3,16 @@ grantpt() function: Acquire ownership of the slave 
side of a pseudo-terminal.

 Files:
 lib/grantpt.c
+lib/pt_chown.c
+lib/pty-private.h
 m4/grantpt.m4

 Depends-on:
 stdlib
 extensions
-pt_chown        [test $HAVE_GRANTPT = 0]
 waitpid         [test $HAVE_GRANTPT = 0]
 configmake      [test $HAVE_GRANTPT = 0]
+ptsname         [test $HAVE_GRANTPT = 0]

 configure.ac:
 gl_FUNC_GRANTPT
@@ -18,9 +20,15 @@ if test $HAVE_GRANTPT = 0; then
   AC_LIBOBJ([grantpt])
   gl_PREREQ_GRANTPT
 fi
+AM_CONDITIONAL([GL_GENERATE_PT_CHOWN], [test $HAVE_GRANTPT = 0])
 gl_STDLIB_MODULE_INDICATOR([grantpt])

 Makefile.am:
+if GL_GENERATE_PT_CHOWN
+# TODO: Add rules for installing as setuid root (chown root, chmod 
a=rx,u+s).
+pkglibexec_PROGRAMS = pt_chown
+pt_chown_LDADD = libgnu.a
+endif

 Include:
 <stdlib.h>
diff --git a/modules/pt_chown b/modules/pt_chown
deleted file mode 100644
index 515df4b..0000000
--- a/modules/pt_chown
+++ /dev/null
@@ -1,26 +0,0 @@
-Description:
-Helper program for setting the owner of the slave side of a 
pseudo-terminal.
-
-Files:
-lib/pt_chown.c
-lib/pty-private.h
-
-Depends-on:
-ptsname
-stdlib
-configmake
-
-configure.ac:
-
-Makefile.am:
-# TODO: Add rules for installing as setuid root (chown root, chmod 
a=rx,u+s).
-pkglibexec_PROGRAMS = pt_chown
-pt_chown_LDADD = libgnu.a
-
-Include:
-
-License:
-LGPLv2+
-
-Maintainer:
-glibc
--
1.7.4.4


From 5f7493c55e00619a70b827098343e29edb7779b9 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 19 Oct 2011 16:23:18 -0600
Subject: [PATCH 2/2] pt_chown: break gnulib link dependency

If pt_chown calls ptsname(), then it must pull in gnulib; but then
we must decide whether libtool is in use (libgnu.a vs. libgnu.la).
Simpler is just shifting the burden to the sole caller.  Even though
this breaks compatibility with glibc pt_chown, we already document
that our helper app is only conditionally built; and since it lives
in pkglibexecdir, no one else should be calling it, anyways.

* lib/pt_chown.c (main, do_pt_chown): Require pty name as argument.
* lib/grantpt.c (grantpt): Update caller.
* modules/grantpt (Makefile.am): Simplify LDADD accordingly.
* NEWS: Document the call convention change.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog       |    6 ++++++
 NEWS            |    3 ++-
 lib/grantpt.c   |    7 ++++++-
 lib/pt_chown.c  |   26 +++++++++++---------------
 modules/grantpt |    1 -
 5 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 49524c2..42b2969 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2011-10-19  Eric Blake  <address@hidden>

+       pt_chown: break gnulib link dependency
+       * lib/pt_chown.c (main, do_pt_chown): Require pty name as argument.
+       * lib/grantpt.c (grantpt): Update caller.
+       * modules/grantpt (Makefile.am): Simplify LDADD accordingly.
+       * NEWS: Document the call convention change.
+
        grantpt: only build pt_chown when needed
        * modules/pt_chown: Delete, merging into...
        * modules/grantpt: ...its sole user.  Make building of pt_chown
diff --git a/NEWS b/NEWS
index 493a25b..0bff6a7 100644
--- a/NEWS
+++ b/NEWS
@@ -15,7 +15,8 @@ Date        Modules         Changes
 2011-10-19  pt_chown        This module no longer exists.  The helper
                             application pt_chown is now only built if 
needed
                             for the 'grantpt' module, since it has no use
-                            as a stand-alone application.
+                            as a stand-alone application; also, it
+                            intentionally differs from glibc's pt_chown.

 2011-10-03  poll            The link requirements of this module are 
changed
                             from empty to $(LIB_POLL).
diff --git a/lib/grantpt.c b/lib/grantpt.c
index 985e31d..1398026 100644
--- a/lib/grantpt.c
+++ b/lib/grantpt.c
@@ -55,6 +55,10 @@ grantpt (int fd)
   else if (pid == 0)
     {
       /* This is executed in the child process.  */
+      char *pty;
+      pty = ptsname (PTY_FILENO);
+      if (pty == NULL)
+        _exit (errno == EBADF ? FAIL_EBADF : FAIL_EINVAL);

 #if HAVE_SETRLIMIT && defined RLIMIT_CORE
       /* Disable core dumps.  */
@@ -71,7 +75,8 @@ grantpt (int fd)
       CLOSE_ALL_FDS ();
 #endif

-      execle (_PATH_PT_CHOWN, strrchr (_PATH_PT_CHOWN, '/') + 1, NULL, 
NULL);
+      execle (_PATH_PT_CHOWN, strrchr (_PATH_PT_CHOWN, '/') + 1, pty, NULL,
+              NULL);
       _exit (FAIL_EXEC);
     }
   else
diff --git a/lib/pt_chown.c b/lib/pt_chown.c
index ccc04fd..f654b2d 100644
--- a/lib/pt_chown.c
+++ b/lib/pt_chown.c
@@ -30,22 +30,18 @@
 /* For security reasons, we try to minimize the dependencies on libraries
    outside libc.  This means, in particular:
      - No use of gettext(), since it's usually implemented in libintl.
-     - No use of error() or argp, since they rely on gettext by 
default.  */
+     - No use of error() or argp, since they rely on gettext by default.
+     - No use of ptsname(), since this implementation is only compiled
+       by gnulib, which implies ptsname is also implemented by gnulib.  */


 static int
-do_pt_chown (void)
+do_pt_chown (char *pty)
 {
-  char *pty;
   struct stat st;
   struct group *p;
   gid_t gid;

-  /* Check that PTY_FILENO is a valid master pseudo terminal.  */
-  pty = ptsname (PTY_FILENO);
-  if (pty == NULL)
-    return errno == EBADF ? FAIL_EBADF : FAIL_EINVAL;
-
   /* Check that the returned slave pseudo terminal is a
      character device.  */
   if (stat (pty, &st) < 0 || !S_ISCHR (st.st_mode))
@@ -75,11 +71,11 @@ main (int argc, char *argv[])
 {
   uid_t euid = geteuid ();

-  if (argc == 1 && euid == 0)
+  if (argc == 2 && argv[1][0] != '-' && euid == 0)
     {
       /* Normal invocation of this program is with no arguments and
          with privileges.  */
-      return do_pt_chown ();
+      return do_pt_chown (argv[1]);
     }

   /* It would be possible to drop setuid/setgid privileges here.  But 
it is not
@@ -123,11 +119,11 @@ main (int argc, char *argv[])

     if (do_help)
       {
-        printf ("Usage: pt_chown [OPTION...]\n");
+        printf ("Usage: pt_chown [OPTION...] PTSNAME\n");
         printf ("Set the owner, group and access permission of the 
slave pseudo terminal\n"
-                "corresponding to the master pseudo terminal passed on 
file descriptor %d.\n"
-                "This is the helper program for the 'grantpt' function. 
 It is not intended\n"
-                "to be run directly from the command line.\n",
+                "PTSNAME corresponding to the master pseudo terminal 
passed on file\n"
+                "descriptor %d.  This is the helper program for the 
'grantpt' function.\n"
+                "It is not intended to be run directly from the command 
line.\n",
                 PTY_FILENO);
         printf ("\n");
         printf ("  --help                     Give this help list\n");
@@ -146,7 +142,7 @@ main (int argc, char *argv[])
         printf ("Copyright (C) %s Free Software Foundation, Inc.\n"
                 "This is free software; see the source for copying 
conditions.  There is NO\n"
                 "warranty; not even for MERCHANTABILITY or FITNESS FOR 
A PARTICULAR PURPOSE.\n",
-                "1999");
+                "2011");
         return EXIT_SUCCESS;
       }
   }
diff --git a/modules/grantpt b/modules/grantpt
index f317faf..51f347f 100644
--- a/modules/grantpt
+++ b/modules/grantpt
@@ -27,7 +27,6 @@ Makefile.am:
 if GL_GENERATE_PT_CHOWN
 # TODO: Add rules for installing as setuid root (chown root, chmod 
a=rx,u+s).
 pkglibexec_PROGRAMS = pt_chown
-pt_chown_LDADD = libgnu.a
 endif

 Include:
--
1.7.4.4

--
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org