[v2] Create block-local FD state when appending redirection closing the FD

Commit Message

Martijn Dekker Dec. 14, 2018, 1:01 a.m. UTC

[Corrected patch due to an oops in first take. Apologies.]

Op 19-09-18 om 05:25 schreef Herbert Xu:
> Harald van Dijk <[e-mail]> 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

Patch

diff --git a/src/redir.c b/src/redir.c
index e67cc0a..1e3feac 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,7 +347,6 @@  popredir(int drop)
 				close(i);
 			break;
 		case EMPTY:
-		case REALLY_CLOSED:
 			break;
 		default:
 			if (!drop)