<>; redirection operator doesn't work for the last command of a ksh -c inline-script
#9
Comments
|
@stephane-chazelas Sorry for taking so long to respond to this issue. This is what other shells do: bash (devel branch): zsh (zsh 5.5.1): So this behavior is compatible with other shells. What's more interesting is |
|
Ok, I see how So this is a bug. |
|
Yes, this is technically a bug. But it has probably been broken since it was introduced on 2009-01-23 for the ksh93t+ release. From the lib/package/ast-open.README file:
It's hard to know if it was broken in the ksh93t+ release that introduced this feature. Or by a subsequent change in the ksh93u+ release. I can't readily test the former version but it is broken in the most recent public release. The fact this idiom is used in the src/cmd/ksh93/tests/io.sh unit test, and that test passes, suggests this feature has never been unit tested. Too, I don't understand how this would be meaningfully used in a ksh script. Yes, I have written lots of code that opens a file, updates its content, and truncates it to a specific length. But I have never expected to be able to do this in a shell script. What is the probability anyone is using this feature? It seems to me to be zero or so close to zero that we don't care. FWIW, I would be happy to retain this feature if it weren't for the fact the code has so many bugs involving its core functionality that support for such fringe features is hard to justify. |
|
This is another bug hiding behind non-forking subshells code. Commenting out this condition fixes this issue. |
|
If this code was in better shape, I would have been willing to maintain non-forking subshells. As we move forward, I am getting less convinced that I am willing to take that effort. |
It's useful to edit a file in place when the editing doesn't add data (like with See https://unix.stackexchange.com/a/265857 For some examples. (in the first one, it's used as just a truncating operator in combination with |
Non-forking subshells have always caused flakey behavior, so we want to reduce their use. Since commands passed with `-c` option are mostly short, performance regressions are unlikely to be noticeable. Resolves: #9
|
@stephane-chazelas Thanks again for bringing this issue to our attention. It's a shame that it took more than 2 years to fix this bug. But as you can see in several other bugs reports, we are fixing bugs that are close to 2 decades old. Despite of all the problems we are dealing with, I still remain optimistic and I would love to see more comments from people like you who have solid understanding of POSIX internals. |
Add a test case for `<>;` with `-c` option See att#9
Add a test case for `<>;` with `-c` option See att#9
On 16 June there was a call for volunteers to fix the bash compatibility mode; it has never successfully compiled in 93u+. Since no one showed up, it is now removed due to lack of interest. A couple of things are kept, which are now globally enabled: 1. The &>file redirection shorthand (for >file 2>&1). As a matter of fact, ksh93 already supported this natively, but only while running rc/profile/login scripts, and it issued a warning. This makse it globally available and removes the warning, bringing ksh93 in line with mksh, bash and zsh. 2. The '-o posix' standard compliance option. It is now enabled on startup if ksh is invoked as 'sh' or if the POSIXLY_CORRECT variable exists in the environment. To begin with, it disables the aforementioned &> redirection shorthand. Further compliance tweaks will be added in subsequent commits. The differences will be fairly minimal as ksh93 is mostly compliant already. In all changed files, code was removed that was compiled (more precisely, failed to compile/link) if the SHOPT_BASH preprocessor identifier was defined. Below are other changes worth mentioning: src/cmd/ksh93/sh/bash.c, src/cmd/ksh93/data/bash_pre_rc.sh: - Removed. src/cmd/ksh93/data/lexstates.c, src/cmd/ksh93/include/shlex.h, src/cmd/ksh93/sh/lex.c: - Globally enable &> redirection operator if SH_POSIX not active. - Remove warning that was issued when &> was used in rc scripts. src/cmd/ksh93/data/options.c, src/cmd/ksh93/include/defs.h, src/cmd/ksh93/sh/args.c: - Keep SH_POSIX option (-o posix). - Replace SH_TYPE_BASH shell type by SH_TYPE_POSIX. src/cmd/ksh93/sh/init.c: - sh_type(): Return SH_TYPE_POSIX shell type if ksh was invoked as sh (or rsh, restricted sh). - sh_init(): Enable posix option if the SH_TYPE_POSIX shell type was detected, or if the CONFORMANCE ast config variable was set to "standard" (which libast sets on init if POSIXLY_CORRECT exists in the environment). src/cmd/ksh93/tests/options.sh, src/cmd/ksh93/tests/io.sh: - Replace regression tests for &> and move to io.sh. Since &> is now for general use, no longer test in an rc script, and don't check that a warning is issued. Closes: att#9 Progresses: att#20
This bug was first reported at att#9. The <>; operator doesn't work correctly if it's used as the last command of a -c script. Reproducer: $ echo test > a; ksh -c 'echo x 1<>; a'; cat a x st This bug is caused by ksh running the last command of -c scripts with execve(2) instead of posix_spawn(3) or fork(2). The <>; operator is noted by the man page as being incompatible with the exec builtin (see also the ksh93u+ man page), so it's not surprising this bug occurs when ksh runs a command using execve: > <>;word cannot be used with the exec and redirect built-ins. The ksh2020 fix simply removed the code required for ksh to use this optimization at all. It's not a performance friendly fix and only papers over the bug, so this commit provides a better fix. src/cmd/ksh93/sh/xec.c: - The IOREWRITE flag is set when handling the <>; operator, so to fix this bug avoid exec'ing the last command if it uses <>;. See also commit 17ebfbf, which fixed another issue related to the execve optimization. src/cmd/ksh93/tests/io.sh: - Enable a regression test that was failing because of this bug. - Add the reproducer from att#9 as a regression test. src/cmd/ksh93/tests/options.sh: - This bugfix was causing the options regression test to segfault when run under shcomp. The cause is the same as ksh93#165, so as a workaround avoid parsing the command substitution with shcomp. This workaround should also avoid the other problem detailed in ksh93#274.
The <>; operator doesn't work correctly if it's used as the last command of a -c script. Reproducer: $ echo test > a; ksh -c 'echo x 1<>; a'; cat a x st This bug is caused by ksh running the last command of -c scripts with execve(2) instead of posix_spawn(3) or fork(2). The <>; operator is noted by the man page as being incompatible with the exec builtin (see also the ksh93u+ man page), so it's not surprising this bug occurs when ksh runs a command using execve: > <>;word cannot be used with the exec and redirect built-ins. The ksh2020 fix simply removed the code required for ksh to use this optimization at all. It's not a performance friendly fix and only papers over the bug, so this commit provides a better fix. This bug was first reported at: att#9 In addition, this commit re-enables the execve(2) optimization for the last command for scripts loaded from a file. It was enabled in in older ksh versions, and was only disabled in interactive shells: https://github.com/ksh93/ast-open-history/blob/2011-06-30/src/cmd/ksh93/sh/main.c#L593-L599 It was changed on 2011-12-24 to only be used for -c scripts: https://github.com/ksh93/ast-open-history/blob/2011-12-24/src/cmd/ksh93/sh/main.c#L593-L599 We think there is no good reason why scripts loaded from a file should be optimised less than scripts loaded from a -c argument. They're both scripts; there's no essential difference between them. So this commit reverts that change. If there is a bug left in the optimization after this fix, this revert increases the chance of exposing it so that it can be fixed. src/cmd/ksh93/sh/xec.c: - The IOREWRITE flag is set when handling the <>; operator, so to fix this bug, avoid exec'ing the last command if it uses <>;. See also commit 17ebfbf, which fixed another issue related to the execve optimization. src/cmd/ksh93/tests/io.sh: - Enable a regression test that was failing because of this bug. - Add the reproducer from att#9 as a regression test. src/cmd/ksh93/sh/main.c: - Only avoid the non-forking optimization in interactive shells. src/cmd/ksh93/tests/signal.sh: - Add an extra comment to avoid the non-forking optimization in the regression test for rhbz#1469624. - If the regression test for rhbz#1469624 fails, show the incorrect exit status in the error message. src/cmd/ksh93/tests/builtins.sh, src/cmd/ksh93/tests/options.sh: - This bugfix was causing the options regression test to segfault when run under shcomp. The cause is the same as <#165>, so as a workaround, avoid parsing process substitutions with shcomp until that is fixed. This workaround should also avoid the other problem detailed in <#274>. Resolves: #274
It's OK, if we insert a command after that:
The text was updated successfully, but these errors were encountered: