[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dash
Subject:    Re: [BUG] failure to push/restore closed file descriptor
From:       Martijn Dekker <martijn () inlv ! org>
Date:       2018-12-14 0:45:38
Message-ID: 818904e8-9bbd-a343-bc95-5a66c19117d4 () inlv ! org
[Download RAW message or body]

Op 19-09-18 om 05:25 schreef Herbert Xu:
> Harald van Dijk <harald@gigawatt.nl> wrote:
>> On 23/04/2018 19:56, Martijn Dekker wrote:
>>> $ dash -c '{ exec 8</dev/null; } 8<&-; : <&8 && echo "oops, still open"'
>>> Output: "oops, still open"
>>> Expected output: Bad file descriptor
>>>
>>> Apparently, dash either fails to push the file descriptor onto the stack
>>> at '} 8<&-', or fails to restore it.
>>>
>>> Same bug with loops ending in "done 8<&-".
>>>
>>> Confirmed in all dash versions down to 0.5.5.1.
>>
>> What surprises me most is that dash has code written specifically to
>> keep the fd closed. dash would be smaller and simpler if it behaved the
>> way you expected and the way most other shells behave: just remove all
>> traces of REALLY_CLOSED.

Probably a lingering case of bug-compatibility with the original Bourne 
shell, which behaves this way (confirmed on a VM with 1988 Xenix).

> So did anything happen on the bash front? I'm happy to change if
> bash moves in the same direction.

Yes. According to the bash changelog, Chet fixed it in git last 30th of 
April, meaning it'll be in bash 5.0.

Patch attached, as per Harald's suggestion.

- Martijn

["BUG_SCLOSEDFD.patch" (text/plain)]

diff --git a/src/redir.c b/src/redir.c
index e67cc0a..03b43c8 100644
--- a/src/redir.c
+++ b/src/redir.c
@@ -57,7 +57,6 @@
 #include "error.h"
 
 
-#define REALLY_CLOSED -3	/* fd that was closed and still is */
 #define EMPTY -2		/* marks an unused slot in redirtab */
 #define CLOSED -1		/* fd opened for redir needs to be closed */
 
@@ -136,10 +135,6 @@ redirect(union node *redir, int flags)
 				}
 			}
 
-			if (i == newfd)
-				/* Can only happen if i == newfd == CLOSED */
-				i = REALLY_CLOSED;
-
 			*p = i;
 		}
 
@@ -352,8 +347,6 @@ popredir(int drop)
 				close(i);
 			break;
 		case EMPTY:
-		case REALLY_CLOSED:
-			break;
 		default:
 			if (!drop)
 				dup2(rp->renamed[i], i);


[prev in list] [next in list] [prev in thread] [next in thread]