File tcsh-closem.patch of Package tcsh.14996

[PATCH] Slightly less drastic closem()
Miloslav Trmac mitr at volny.cz
Thu Sep 9 19:17:10 EDT 2004

    Previous message: [PATCH] Slightly less drastic closem()
    Next message: Newlines in command substitution
    Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Sep 08, 2004 at 08:38:03PM -0400, Christos Zoulas wrote:
(Original mail reordered)
> Finally, I do have an old patch that makes tcsh not use closem at all,
> and adds sh like I/O redirection (plus other fd manipulations) to it,
sh-like I/O redirection could actually make the problem worse if
tcsh is using NSS after performing the redirections --- I haven't checked
whether it does.

Avoiding closem () completely is of course the "correct" fix, but I'm
afraid I won't be able to spend the time needed to do this.


> The same problem has been present for years with NIS. Same as with nss_ldap,
> NIS happily discovers that the fd it was using before is gone, and re-opens
> it.
Are other file types than sockets involved?

> I don't like the socket hack because:
> 
> 1. It makes closem not close all the fds anymore.
I have already argued this should not be a problem: tcsh never
creates sockets.

> 3. the message that nss_ldap produces should be log_debug at best.
I can imagine scenarios where the message is really useful, but
I don't feel strongly about it.

> 2. The fact that nss_ldap uses sockets now is an artifact of the
>    implementation. What if tomorrow it changes to use doors or named pipes?
I have never met doors in practice so I won't comment on them except by
noting that the nss_ldap in question surely won't use them.

If nss_ldap used named pipes, the "it is necessary for performance"
justification would be so weak that I would prefer patching nss_ldap
to close the pipe after each call instead of changing tcsh.

nss_ldap and tcsh are fighting over a gray area in the
system <-> application contract, so it seems reasonable to
solve it by a "compromise", restricting the behavior of both.


AFAICS, solving the general case reliably would require a lot of work
and I have no emprical evidence suggesting that this work is necessary.

To make this lobbying attempt complete :-), I'm attaching a properly
commented socket hack patch.
    Mirek
---
 sh.misc.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

--- sh.misc.c
+++ sh.misc.c	2017-06-16 07:51:59.732399828 +0000
@@ -257,6 +257,7 @@ void
 closem(void)
 {
     int f, num_files;
+    struct stat st;
 
 #ifdef NLS_BUGS
 #ifdef NLS_CATALOGS
@@ -274,6 +275,16 @@ closem(void)
 #ifdef MALLOC_TRACE
 	    && f != 25
 #endif /* MALLOC_TRACE */
+#ifdef S_ISSOCK
+	    /* NSS modules (e.g. Linux nss_ldap) might keep sockets open.
+	     * If we close such a socket, both the NSS module and tcsh think
+	     * they "own" the descriptor.
+	     *
+	     * Not closing sockets does not make the cleanup use of closem()
+	     * less reliable because tcsh never creates sockets.
+	     */
+	    && fstat(f, &st) == 0 && !S_ISSOCK(st.st_mode)	
+#endif
 	    )
 	  {
 	    xclose(f);