* warning about closing an already closed file descriptor
@ 2015-01-19 13:33 Kamil Dudka
2015-01-19 14:59 ` Peter Stephenson
0 siblings, 1 reply; 7+ messages in thread
From: Kamil Dudka @ 2015-01-19 13:33 UTC (permalink / raw)
To: Zsh Users
zsh prints a warning if an already closed file descriptor is to be closed.
While this is technically correct, the other shells I tried (ksh and bash)
do not print any such warning:
$ bash -c '(true <&-) <&-'
$ ksh -c '(true <&-) <&-'
$ zsh -c '(true <&-) <&-'
zsh:1: failed to close file descriptor 0: bad file descriptor
The warning was introduced by the following commit:
http://sourceforge.net/p/zsh/code/ci/45913f43
... and it causes problems when porting legacy scripts to a newer version
of zsh. Is there any way to suppress the warning?
If not, how can one achieve compatibility with other shells (including
older versions of zsh)?
Kamil
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: warning about closing an already closed file descriptor
2015-01-19 13:33 warning about closing an already closed file descriptor Kamil Dudka
@ 2015-01-19 14:59 ` Peter Stephenson
2015-01-19 17:28 ` Peter Stephenson
0 siblings, 1 reply; 7+ messages in thread
From: Peter Stephenson @ 2015-01-19 14:59 UTC (permalink / raw)
To: Zsh Users
On Mon, 19 Jan 2015 14:33:25 +0100
Kamil Dudka <kdudka@redhat.com> wrote:
> zsh prints a warning if an already closed file descriptor is to be closed.
> While this is technically correct, the other shells I tried (ksh and bash)
> do not print any such warning:
>
> $ bash -c '(true <&-) <&-'
> $ ksh -c '(true <&-) <&-'
> $ zsh -c '(true <&-) <&-'
> zsh:1: failed to close file descriptor 0: bad file descriptor
>
> The warning was introduced by the following commit:
> http://sourceforge.net/p/zsh/code/ci/45913f43
>
> ... and it causes problems when porting legacy scripts to a newer version
> of zsh. Is there any way to suppress the warning?
>
> If not, how can one achieve compatibility with other shells (including
> older versions of zsh)?
The problem that change fixed wasn't really intended to have a visible
effect, apart from closing f.d.s the shell didn't know about. If it's
more useful without the message than with it, it can be removed. The
shell doesn't make it particularly convenient to test first, either (you
can test something that isn't quite the point by means that aren't quite
intended for it).
Does anyone want to own up to using the message for debugging?
We can special case the {varid}<&- syntax --- that might be useful since
it implies the user was previously in direct control of the f.d. so is
ikely to be interested in an error if closing it failed.
pws
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: warning about closing an already closed file descriptor
2015-01-19 14:59 ` Peter Stephenson
@ 2015-01-19 17:28 ` Peter Stephenson
2015-01-20 9:36 ` Kamil Dudka
2015-01-20 11:02 ` Roman Neuhauser
0 siblings, 2 replies; 7+ messages in thread
From: Peter Stephenson @ 2015-01-19 17:28 UTC (permalink / raw)
To: Zsh Users
On Mon, 19 Jan 2015 14:59:35 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> The problem that change fixed wasn't really intended to have a visible
> effect, apart from closing f.d.s the shell didn't know about. If it's
> more useful without the message than with it, it can be removed.
>...
> We can special case the {varid}<&- syntax --- that might be useful since
> it implies the user was previously in direct control of the f.d. so is
> ikely to be interested in an error if closing it failed.
This easy patch does that. Is anyone other than Kamil interested enough
to comment?
pws
diff --git a/Src/exec.c b/Src/exec.c
index f42fb2b..3b0e936 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3203,7 +3203,12 @@ execcmd(Estate state, int input, int output, int how, int last1)
}
if (fn->fd1 < 10)
closemn(mfds, fn->fd1, REDIR_CLOSE);
- if (!closed && zclose(fn->fd1) < 0) {
+ /*
+ * Only report failures to close file descriptors
+ * if they're under user control as we don't know
+ * what the previous status of others was.
+ */
+ if (!closed && zclose(fn->fd1) < 0 && fn->varid) {
zwarn("failed to close file descriptor %d: %e",
fn->fd1, errno);
}
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index a39ce46..cb67788 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -152,11 +152,13 @@
>hello
>goodbye
- ({ exec 3<&- } 2>/dev/null
- exec 3<&-
- read foo <&-)
+ (exec {varid}<&0
+ exec {varid}<&-
+ print About to close a second time >&2
+ read {varid}<&-)
1:'<&-' redirection
-*?\(eval\):*: failed to close file descriptor 3:*
+?About to close a second time
+*?\(eval\):*: failed to close file descriptor *
print foo >&-
0:'>&-' redirection
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: warning about closing an already closed file descriptor
2015-01-19 17:28 ` Peter Stephenson
@ 2015-01-20 9:36 ` Kamil Dudka
2015-01-20 11:02 ` Roman Neuhauser
1 sibling, 0 replies; 7+ messages in thread
From: Kamil Dudka @ 2015-01-20 9:36 UTC (permalink / raw)
To: Peter Stephenson; +Cc: zsh-users
On Monday 19 January 2015 17:28:50 Peter Stephenson wrote:
> On Mon, 19 Jan 2015 14:59:35 +0000
>
> Peter Stephenson <p.stephenson@samsung.com> wrote:
> > The problem that change fixed wasn't really intended to have a visible
> > effect, apart from closing f.d.s the shell didn't know about. If it's
> > more useful without the message than with it, it can be removed.
> >
> >...
> >
> > We can special case the {varid}<&- syntax --- that might be useful since
> > it implies the user was previously in direct control of the f.d. so is
> > ikely to be interested in an error if closing it failed.
>
> This easy patch does that. Is anyone other than Kamil interested enough
> to comment?
>
> pws
>
> diff --git a/Src/exec.c b/Src/exec.c
> index f42fb2b..3b0e936 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -3203,7 +3203,12 @@ execcmd(Estate state, int input, int output, int how,
> int last1) }
> if (fn->fd1 < 10)
> closemn(mfds, fn->fd1, REDIR_CLOSE);
> - if (!closed && zclose(fn->fd1) < 0) {
> + /*
> + * Only report failures to close file descriptors
> + * if they're under user control as we don't know
> + * what the previous status of others was.
> + */
> + if (!closed && zclose(fn->fd1) < 0 && fn->varid) {
> zwarn("failed to close file descriptor %d: %e",
> fn->fd1, errno);
> }
Looks reasonable and fixes the problem for me. Thanks for the patch!
Kamil
> diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
> index a39ce46..cb67788 100644
> --- a/Test/A04redirect.ztst
> +++ b/Test/A04redirect.ztst
> @@ -152,11 +152,13 @@
>
> >hello
> >goodbye
>
> - ({ exec 3<&- } 2>/dev/null
> - exec 3<&-
> - read foo <&-)
> + (exec {varid}<&0
> + exec {varid}<&-
> + print About to close a second time >&2
> + read {varid}<&-)
> 1:'<&-' redirection
> -*?\(eval\):*: failed to close file descriptor 3:*
> +?About to close a second time
> +*?\(eval\):*: failed to close file descriptor *
>
> print foo >&-
> 0:'>&-' redirection
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: warning about closing an already closed file descriptor
2015-01-19 17:28 ` Peter Stephenson
2015-01-20 9:36 ` Kamil Dudka
@ 2015-01-20 11:02 ` Roman Neuhauser
2015-01-20 11:39 ` Peter Stephenson
1 sibling, 1 reply; 7+ messages in thread
From: Roman Neuhauser @ 2015-01-20 11:02 UTC (permalink / raw)
To: Peter Stephenson; +Cc: Zsh Users
# p.stephenson@samsung.com / 2015-01-19 17:28:50 +0000:
> On Mon, 19 Jan 2015 14:59:35 +0000
> Peter Stephenson <p.stephenson@samsung.com> wrote:
> > The problem that change fixed wasn't really intended to have a visible
> > effect, apart from closing f.d.s the shell didn't know about. If it's
> > more useful without the message than with it, it can be removed.
> >...
> > We can special case the {varid}<&- syntax --- that might be useful since
> > it implies the user was previously in direct control of the f.d. so is
> > ikely to be interested in an error if closing it failed.
>
> This easy patch does that. Is anyone other than Kamil interested enough
> to comment?
> diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
> index a39ce46..cb67788 100644
> --- a/Test/A04redirect.ztst
> +++ b/Test/A04redirect.ztst
> @@ -152,11 +152,13 @@
> >hello
> >goodbye
>
> - ({ exec 3<&- } 2>/dev/null
> - exec 3<&-
> - read foo <&-)
> + (exec {varid}<&0
> + exec {varid}<&-
> + print About to close a second time >&2
> + read {varid}<&-)
sorry if i'm talking out of my ass, but does this mean that
the non-{varid} syntax is now without a test?
> 1:'<&-' redirection
> -*?\(eval\):*: failed to close file descriptor 3:*
> +?About to close a second time
> +*?\(eval\):*: failed to close file descriptor *
>
> print foo >&-
> 0:'>&-' redirection
--
roman
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: warning about closing an already closed file descriptor
2015-01-20 11:02 ` Roman Neuhauser
@ 2015-01-20 11:39 ` Peter Stephenson
2015-01-20 11:43 ` Roman Neuhauser
0 siblings, 1 reply; 7+ messages in thread
From: Peter Stephenson @ 2015-01-20 11:39 UTC (permalink / raw)
Cc: Zsh Users
On Tue, 20 Jan 2015 12:02:03 +0100
Roman Neuhauser <neuhauser@sigpipe.cz> wrote:
> > - ({ exec 3<&- } 2>/dev/null
> > - exec 3<&-
> > - read foo <&-)
> > + (exec {varid}<&0
> > + exec {varid}<&-
> > + print About to close a second time >&2
> > + read {varid}<&-)
>
> sorry if i'm talking out of my ass, but does this mean that
> the non-{varid} syntax is now without a test?
For closing fd's, yes. There's no output to test --- though there is a
status, so we could still test that. And I suppose the lack of an error
message is itself a test...
pws
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index cb67788..13f1f7c 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -152,11 +152,16 @@
>hello
>goodbye
+ ({exec 3<&- } 2>/dev/null
+ exec 3<&-
+ read foo <&-)
+1:'<&-' redirection with numeric fd (no error message on failure)
+
(exec {varid}<&0
exec {varid}<&-
print About to close a second time >&2
read {varid}<&-)
-1:'<&-' redirection
+1:'<&-' redirection with fd in variable (error message on failure)
?About to close a second time
*?\(eval\):*: failed to close file descriptor *
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: warning about closing an already closed file descriptor
2015-01-20 11:39 ` Peter Stephenson
@ 2015-01-20 11:43 ` Roman Neuhauser
0 siblings, 0 replies; 7+ messages in thread
From: Roman Neuhauser @ 2015-01-20 11:43 UTC (permalink / raw)
To: Peter Stephenson
# p.stephenson@samsung.com / 2015-01-20 11:39:43 +0000:
> On Tue, 20 Jan 2015 12:02:03 +0100
> Roman Neuhauser <neuhauser@sigpipe.cz> wrote:
> > > - ({ exec 3<&- } 2>/dev/null
> > > - exec 3<&-
> > > - read foo <&-)
> > > + (exec {varid}<&0
> > > + exec {varid}<&-
> > > + print About to close a second time >&2
> > > + read {varid}<&-)
> >
> > sorry if i'm talking out of my ass, but does this mean that
> > the non-{varid} syntax is now without a test?
>
> For closing fd's, yes. There's no output to test --- though there is a
> status, so we could still test that. And I suppose the lack of an error
> message is itself a test...
yup, testing that stderr is actually quiet was my concern. thanks a lot!
--
roman
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-20 11:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-19 13:33 warning about closing an already closed file descriptor Kamil Dudka
2015-01-19 14:59 ` Peter Stephenson
2015-01-19 17:28 ` Peter Stephenson
2015-01-20 9:36 ` Kamil Dudka
2015-01-20 11:02 ` Roman Neuhauser
2015-01-20 11:39 ` Peter Stephenson
2015-01-20 11:43 ` Roman Neuhauser
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).