Re: [PATCH] Add the --with-tty-gid configure option [BZ #24941]

["Gabriel F. T. Gomes" <gabriel at inconstante dot net dot br>]

Hi, Florian,

Thanks for, in the bug report [1], helping me understand the rationale for
this patch (the problem was well described here, anyway, I had just failed
to notice the details).

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=24941#c3

On Tue, 27 Aug 2019, Florian Weimer wrote:

>2019-08-27  Florian Weimer  <fweimer@redhat.com>
>
>	[BZ #24941]
>	Add the --with-tty-gid configure option.
>	* config.make.in (define-tty-gid-cflags): New variable.
>	* configure.ac (--with-tty-gid): New option.
>	* manual/install.texi (Configuring and compiling): Document
>	--with-tty-gid.
>	* sysdeps/unix/Makefile [$(subdir) == login] (CFLAGS-grantpt.c):
>	Add $(define-tty-gid-cflags).
>	* sysdeps/unix/grantpt.c (grantpt): Use TTY_GID constant for gid
>	if defined.
>	* configure, INSTALL: Regeneration.

I thinks this needs a new commit message, preferably one that summarizes
why this patch is useful...  Something in the lines of:

When, in multi-threaded applications, a forked process tries to call
grantpt, it could hang in an internal NSS lock (__nss_database_lookup2),
if the fork happened while NSS was used in another thread.  This patch
adds the --with-tty-gid configure option, which avoids the hang, by
avoiding calls to NSS functions.


Other than that, the patch looks good to me.

Reviewed-by: Gabriel F. T. Gomes <gabrielftg@linux.ibm.com>

Thanks


>+@item --with-tty-gid=@var{GID}
>+Use @var{GID} as the value for the group ID of the @samp{tty} group.
>+This enables a more efficient implementation of the @code{grantpt}
>+function, and the function will also work reliably in a forked
>+subprocess of a multi-threaded process.

OK.

>+#ifdef TTY_GID
>+  /* TTY_GID is set by the --with-tty-gid configure option.  */
>+  enum { gid = TTY_GID };
>+#else
>   static int tty_gid = -1;
>   if (__glibc_unlikely (tty_gid == -1))
>     {
>@@ -154,6 +158,7 @@ grantpt (int fd)
> 	tty_gid = p->gr_gid;
>     }
>   gid_t gid = tty_gid == -1 ? __getgid () : tty_gid;
>+#endif

OK.