* PATCH: coprocess descriptor and error messages @ 2010-09-17 5:04 Bart Schaefer 2010-09-17 8:46 ` Peter Stephenson 0 siblings, 1 reply; 4+ messages in thread From: Bart Schaefer @ 2010-09-17 5:04 UTC (permalink / raw) To: zsh-workers This seems somehow wrong to me ... starting from "zsh -f": torch% print -p ; echo $? print: bad file number: -1 1 torch% read -p ; echo $? 1 Seems to me that both commands should either fail silently, or print a more specific error. Patch below does the former. (Line numbers may be off.) I won't commit until I see commentary. Index: Src/builtin.c =================================================================== RCS file: /extra/cvsroot/zsh/zsh-4.0/Src/builtin.c,v retrieving revision 1.48 diff -c -r1.48 builtin.c --- builtin.c 17 Apr 2009 18:57:22 -0000 1.48 +++ builtin.c 17 Sep 2010 05:02:15 -0000 @@ -3677,13 +3720,17 @@ if (OPT_HASARG(ops,'u') || OPT_ISSET(ops,'p')) { int fd; - if (OPT_ISSET(ops, 'p')) + if (OPT_ISSET(ops, 'p')) { fd = coprocout; - else { + if (fd < 0) + return 1; + } else { char *argptr = OPT_ARG(ops,'u'), *eptr; /* Handle undocumented feature that -up worked */ if (!strcmp(argptr, "p")) { fd = coprocout; + if (fd < 0) + return 1; } else { fd = (int)zstrtol(argptr, &eptr, 10); if (*eptr) { -- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: coprocess descriptor and error messages 2010-09-17 5:04 PATCH: coprocess descriptor and error messages Bart Schaefer @ 2010-09-17 8:46 ` Peter Stephenson 2010-09-17 14:41 ` Bart Schaefer 0 siblings, 1 reply; 4+ messages in thread From: Peter Stephenson @ 2010-09-17 8:46 UTC (permalink / raw) To: zsh-workers On Thu, 16 Sep 2010 22:04:42 -0700 Bart Schaefer <schaefer@brasslantern.com> wrote: > This seems somehow wrong to me ... starting from "zsh -f": > > torch% print -p ; echo $? > print: bad file number: -1 > 1 > torch% read -p ; echo $? > 1 > > Seems to me that both commands should either fail silently, or print a > more specific error. Patch below does the former. (Line numbers may > be off.) I won't commit until I see commentary. The question is whether it's useful to be able to test whether the coprocess is open at the same time as attempting to read or write. If so, doing it silently is reasonable. Otherwise it might be less mysterious to print "coprocess not open", or something like that, in both cases. -- Peter Stephenson <pws@csr.com> Software Engineer Tel: +44 (0)1223 692070 Cambridge Silicon Radio Limited Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: coprocess descriptor and error messages 2010-09-17 8:46 ` Peter Stephenson @ 2010-09-17 14:41 ` Bart Schaefer 2010-09-17 16:11 ` Bart Schaefer 0 siblings, 1 reply; 4+ messages in thread From: Bart Schaefer @ 2010-09-17 14:41 UTC (permalink / raw) To: zsh-workers On Sep 17, 9:46am, Peter Stephenson wrote: } } The question is whether it's useful to be able to test whether the } coprocess is open at the same time as attempting to read or write. Is there another obvious way to test whether the coprocess is open? Aside: Should an interactive shell ever exit on SIGPIPE? Or maybe a better question is, should a print to the coprocess ever cause an un-trapped SIGPIPE? schaefer[713] Src/zsh -f torch% coproc repeat 8 read -E [1] 31339 torch% repeat 10 { print -p; read -p } schaefer[714] echo $? 141 } If so, doing it silently is reasonable. Otherwise it might be less } mysterious to print "coprocess not open", or something like that, } in both cases. In looking further, the coprocess descriptor is reset to -1 only (and immediately) by "read -p" detecting EOF on the coprocess output. So in the "read" case, the invalid descriptor has always served as an EOF flag. What should happen if the parent attempts to read after an EOF has been seen? I would tend to think it should just get EOF again, not produce a diagnostic, which is the current behavior. However, ksh (at least pdksh) disagrees with me: $ print -p ksh: print: -p: no coprocess $ read |& [1] 31399 $ print -p $ print -p ksh: print: -p: no coprocess [1] + Done read $ read -p $ read -p ksh: read: -p: no coprocess $ print -p ksh: print: -p: no coprocess $ Note, however, that it does NOT exit with SIGPIPE on "print -p". ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: coprocess descriptor and error messages 2010-09-17 14:41 ` Bart Schaefer @ 2010-09-17 16:11 ` Bart Schaefer 0 siblings, 0 replies; 4+ messages in thread From: Bart Schaefer @ 2010-09-17 16:11 UTC (permalink / raw) To: zsh-workers On Sep 17, 7:41am, Bart Schaefer wrote: } } Aside: Should an interactive shell ever exit on SIGPIPE? Or maybe } a better question is, should a print to the coprocess ever cause an } un-trapped SIGPIPE? Experimenting with pdksh seems to indicate that the equivalent of zsh's coprocout descriptor is closed in the child-exited handler when the coprocess terminates. I don't recall enough about (and didn't yet delve back into) the zsh job table to know if we have enough information to identify the coprocess job at that point. The coprocin descriptor remains open until EOF, as in zsh. } In looking further, the coprocess descriptor is reset to -1 only (and } immediately) by "read -p" detecting EOF on the coprocess output. So } in the "read" case, the invalid descriptor has always served as an } EOF flag. What should happen if the parent attempts to read after } an EOF has been seen? I would tend to think it should just get EOF } again, not produce a diagnostic, which is the current behavior. } } However, ksh (at least pdksh) disagrees with me: Here's a patch that duplicates the pdksh behavior, except for the part about exiting on SIGPIPE if an extra print occurs before all output has been read. Apply this *instead* of the previous patch. Index: builtin.c =================================================================== RCS file: /extra/cvsroot/zsh/zsh-4.0/Src/builtin.c,v retrieving revision 1.48 diff -c -r1.48 builtin.c --- builtin.c 17 Apr 2009 18:57:22 -0000 1.48 +++ builtin.c 17 Sep 2010 15:59:06 -0000 @@ -3677,13 +3720,21 @@ if (OPT_HASARG(ops,'u') || OPT_ISSET(ops,'p')) { int fd; - if (OPT_ISSET(ops, 'p')) + if (OPT_ISSET(ops, 'p')) { fd = coprocout; - else { + if (fd < 0) { + zwarnnam(name, "-p: no coprocess"); + return 1; + } + } else { char *argptr = OPT_ARG(ops,'u'), *eptr; /* Handle undocumented feature that -up worked */ if (!strcmp(argptr, "p")) { fd = coprocout; + if (fd < 0) { + zwarnnam(name, "-p: no coprocess"); + return 1; + } } else { fd = (int)zstrtol(argptr, &eptr, 10); if (*eptr) { @@ -5038,6 +5099,10 @@ /* The old code handled -up, but that was never documented. Still...*/ if (!strcmp(argptr, "p")) { readfd = coprocin; + if (readfd < 0) { + zwarnnam(name, "-p: no coprocess"); + return 1; + } } else { readfd = (int)zstrtol(argptr, &eptr, 10); if (*eptr) { @@ -5052,6 +5117,10 @@ izle = 0; } else if (OPT_ISSET(ops,'p')) { readfd = coprocin; + if (readfd < 0) { + zwarnnam(name, "-p: no coprocess"); + return 1; + } izle = 0; } else readfd = izle = 0; ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-09-17 16:12 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-09-17 5:04 PATCH: coprocess descriptor and error messages Bart Schaefer 2010-09-17 8:46 ` Peter Stephenson 2010-09-17 14:41 ` Bart Schaefer 2010-09-17 16:11 ` Bart Schaefer
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).