Re: [BUG] failure to push/restore closed file descriptor

2018-12-13 Thread Martijn Dekker

Op 19-09-18 om 05:25 schreef Herbert Xu:

Harald van Dijk  wrote:

On 23/04/2018 19:56, Martijn Dekker wrote:

$ dash -c '{ exec 8

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
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);


Re: [BUG] failure to push/restore closed file descriptor

2018-09-18 Thread Herbert Xu
Harald van Dijk  wrote:
> On 23/04/2018 19:56, Martijn Dekker wrote:
>> $ dash -c '{ exec 8> 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.

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

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Re: [BUG] failure to push/restore closed file descriptor

2018-09-12 Thread Harald van Dijk

On 23/04/2018 19:56, Martijn Dekker wrote:

$ dash -c '{ exec 8Apparently, 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.


FWIW, this also happens with n< which is ignored entirely. The 
behaviour I would expect for that (and which I have implemented, but 
which I am pretty sure has no chance anyway of getting into dash) is to 
issue an error message if that fd is not open, and to save and later 
restore the fd if it is open.


Test cases:

  : 8<&8

Assuming fd 8 does not happen to be open already, bash and dash are the 
only shells I've tested which accept this. ksh93, mksh, pdksh, yash, 
zsh, bosh all reject it.


  echo ok | { { exec 0<&-; } 0<&0; cat; }

Here, bash and dash are in agreement again, but this time they're not 
alone: mksh and posh also cause an error message by leaving stdin 
closed. The other shells restore stdin and print ok.


There is one more corner case on the subject of redirections, which 
behaves unexpectedly in two shells in two different ways:


  echo ok | cat 0>&0

Almost all shells are okay with this. The exceptions are bosh and yash: 
yash rejects the redirection as fd 0 is not writeable, bosh appears to 
mishandle all 0>* redirections, instead treating them as 1>*.


Cheers,
Harald van Dijk


Re: [BUG] failure to push/restore closed file descriptor

2018-04-27 Thread Martijn Dekker

Op 27-04-18 om 17:51 schreef Herbert Xu:

On Fri, Apr 27, 2018 at 05:47:27PM +0200, Martijn Dekker wrote:


No, because step 1 doesn't merely close fd 8. It enters a curly braces block
(a compound command) that locally closes fd 8 using a redirection, just like
any other redirection would be local to that compound command. Thus,
restoring the fd state when leaving that block must undo the effect of the
'exec'.

Note that dash already does this correctly if the '8<&-' is replaced by any
other redirection such as '8>/dev/tty'.


That's different as 8 was previously closed and is now open.


POSIX 2.7 Redirection says: "Redirection is used to open and close files for
the current shell execution environment [...] or for any command". Note that
"any command" includes compound commands such as curly braces blocks.


Well, I don't see anything in POSIX that requires us to close fd 8.
Can you please point it out to me please?


I will ask more authoritative people on the Austin Group for 
clarification and get back to you.


Even if it turns out it's not "required", though, I would think the 
current behaviour breaks obvious expectations.


To guarantee the expected behaviour of pushing that fd onto the shell's 
internal stack for restoring when leaving the compound command, you'd 
need to push it twice in two different states in two nested compound 
command blocks, for instance:


  { { exec 8/dev/null

The need for that workaround seems like evidence of a bug to me.

- M.
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html