Upon encountering two filenames with multibyte characters starting
with the same byte, a partial multibyte character was completed.
Reproducer (to run in UTF-8 locale):
$ touch XXXá XXXë
$ : XX <== pres tab
$ : XXX^? <== partial multibyte character appears
Note: á is $'\xc3\xa1' and ë is $'\xc3\xab' (same initial byte).
src/cmd/ksh93/edit/completion.c:
- Add multibyte support to the charcmp() and overlaid() functions.
Thanks to Harald van Dijk for useful code and suggestions.
- Add a few missing mbinit() calls. The state of multibyte
processing must be reset before starting a new loop in case a
previous processing run was interrupted mid-character.
src/cmd/ksh93/tests/pty.sh:
- Add test based on Harald's reproducer.
Resolves: https://github.com/ksh93/ksh/issues/223
Until now, when performing any tilde expansion like ~/foo or
~user/foo, ksh added a placeholder built-in command called
'.sh.tilde', ostensibly with the intention to allow users to
override it with a shell function or custom builtin. The multishell
ksh93 repo <https://github.com/multishell/ksh93/> shows this was
added sometime between 2002-06-28 and 2004-02-29. However, it has
never worked and crashed the shell.
This commit replaces that with something that works. Specific tilde
expansions can now be overridden using .set or .get discipline
functions associated with the .sh.tilde variable (see manual,
Discipline Functions).
For example, you can use either of:
.sh.tilde.set()
{
case ${.sh.value} in
'~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;;
'~doc') .sh.value=~/Documents ;;
'~ksh') .sh.value=/usr/local/src/ksh93/ksh ;;
esac
}
.sh.tilde.get()
{
case ${.sh.tilde} in
'~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;;
'~doc') .sh.value=~/Documents ;;
'~ksh') .sh.value=/usr/local/src/ksh93/ksh ;;
esac
}
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/data/variables.c:
- Add SH_TILDENOD for a new ${.sh.tilde} predefined variable.
It is initially unset.
src/cmd/ksh93/sh/macro.c:
- sh_btilde(): Removed.
- tilde_expand2(): Rewritten. I started out with the tiny version
of this function from the 2002-06-28 version of ksh. It uses the
stack instead of sfio, which is more efficient. A bugfix for
$HOME == '/' was retrofitted so that ~/foo does not become
//foo instead of /foo. The rest is entirely new code.
To implement the override functionality, it now checks if
${.sh.tilde} has any discipline function associated with it.
If it does, it assigns the tilde expression to ${.sh.tilde} using
nv_putval(), triggering the .set discipline, and then reads it
back using nv_getval(), triggering the .get discipline. The
resulting value is used if it is nonempty and does not still
start with a tilde.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/builtins.sh:
- Since ksh no longer adds a dummy '.sh.tilde' builtin, remove the
ad-hoc hack that suppressed it from the output of 'builtin'.
src/cmd/ksh93/tests/tilde.sh:
- Add tests verifying everything I can think of, as well as tests
for bugs found and fixed during this rewrite.
src/cmd/ksh93/tests/pty.sh:
- Add test verifying that the .sh.tilde.set() discipline does not
modify the exit status value ($?) when performing tilde expansion
as part of tab completion.
src/cmd/ksh93/sh.1:
- Instead of "tilde substitution", call the basic mechanism "tilde
expansion", which is the term used everywhere else (including the
1995 Bolsky/Korn ksh book).
- Document the new override feature.
Resolves: https://github.com/ksh93/ksh/issues/217
A discipline function could incorrectly influence the value of $?
(exit status of last command) outside its context if it was
triggered without another command being run, e.g. when a prompt
variable is read, or COLUMNS or LINES is set.
Reproducers include:
PS1 prompt:
$ PS1.get() { true; }
$ false
$ echo $?
0
PS2 prompt:
$ PS2.get() { return 13; }
$ \
>
$ echo $?
13
The set discipline is affected too, e.g. COLUMNS and LINES:
$ COLUMNS.set() { return 13; }
$ true
$ (press return)
$ echo $?
13
There are probably other contexts where the shell reads or changes
variables without running commands, allowing their get or set
disciplines to influence $?. So this commit makes ksh save $? for
all .get, .set, .append, and .unset discipline calls.
src/cmd/ksh93/sh/nvdisc.c:
- assign(): Save/restore $? when running a .set/.append/.unset
discipline function.
- lookup(): Save/restore $? when running a .get discipline.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for $? after displaying a prompt
and when setting a LINES.set discipline function.
src/cmd/ksh93/tests/return.sh:
- The above test fails in script form on ksh93u+ and ksh2020, as
it exposes another form of #117 that occurs after running a
subshell. Add the above regression test here as well
(re: 092b90da).
Co-authored-by: Martijn Dekker <martijn@inlv.org>
For most numeric types the last provided one wins out. This commit
closes the gap for -F and -i numerics to not be covered up by other
preceding float types. Note: -u for requesting an unsigned float or
integer was considered and decided to be left alone as it stands,
so as to not allow the variable to become an uppercased string if
the requested options ended with a -u. As it stands for a case when
multiple numeric types are requested, a -u option may be applied
after the last numeric type is processed.
Examples:
-EF becomes -F
-Fi becomes -i
-Fu becomes -F
-uF becomes -F
-Fui becomes -i (because isfloat==1, unsigned is not applied)
-Fiu becomes -iu (isfloat is reset and allows unsigned to be set)
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Reset attribute bit flags for -E and -X when -F is requested by
adding in NV_EXPNOTE to be removed.
- For -i option if a float precedes it, reset isfloat and -E/-F
attribute bit flags.
- Take into account the impact of the shortint flag on floats.
src/cmd/ksh93/tests/attributes.sh:
- Add some validation tests to confirm that, when a -F follows
either -E or -X, -F is used.
- Add some validation tests to confirm that, when -F/E/X precede
a -i, the variable becomes an integer and not a float.
- Add in various tests when -s followed a float.
There was an issue with tilde expansion if the HOME var is unset.
$ unset HOME
$ echo ~
martijn
Only the username is returned. Users are more likely to expect the
current user's home directory as configured in the OS.
POSIXly, the expansion of ~ is based on the value of HOME. If HOME
is unset, the results are unspecified. After unsetting HOME, in
bash, ~ returns the user's home directory as specified by the OS,
whereas in all other shells, ~ expands to the empty string. Only
ksh93 returns the username. The behaviour of bash is more useful.
Discussion:
https://github.com/ksh93/ksh/pull/225#issuecomment-799074107
src/cmd/ksh93/sh/macro.c,
src/cmd/ksh93/tests/tilde.sh:
- sh_tilde(): Backport fix by Mike Gilbert from ksh2020.
See: https://github.com/att/ast/issues/1391https://github.com/att/ast/pull/1396https://github.com/att/ast/commit/070d365d
- Add test.
src/cmd/ksh93/COMPATIBILITY:
- Note this change.
This commit fixes a long-standing bug (present since at least
ksh93r) that caused a file descriptor leak when passing a process
substitution to a function, or (if compiled with SHOPT_SPAWN) to a
nonexistent command.
The leaks only occurred when ksh was compiled with SHOPT_DEVFD; the
FIFO method was unaffected.
src/cmd/ksh93/sh/xec.c: sh_exec():
- When a process substitution is passed to a built-in, the
remaining file descriptor is closed with sh_iorestore. Do the
same thing when passing a process substitution to a function.
This is done by delaying the sh_iorestore() call to 'setexit:'
where both built-ins and functions terminate and set the exit
status ($?).
This means that call now will not be executed if a longjmp is
done, e.g. due to an error in a special built-in. However, there
is already another sh_iorestore() call in main.c, exfile(), line
418, that handles that scenario.
- sh_ntfork() can fail, so rather than assume it will succeed,
handle a failure by closing extra file descriptors with
sh_iorestore(). This fixes the leak on command not found with
SHOPT_SPAWN.
src/cmd/ksh93/include/defs.h:
- Since the file descriptor leaks are now fixed, remove the
workaround that forced ksh to use the FIFO method.
src/cmd/ksh93/SHOPT.sh:
- Add SHOPT_DEVFD as a configurable option (default: probe).
src/cmd/ksh93/tests/io.sh:
- Add a regression test for the 'not found' file descriptor leak.
- Add a test to ensure it keeps working with 'command'.
Fixes: https://github.com/ksh93/ksh/issues/67
On systems where ksh needs to use the older and less secure FIFO
method for process substitutions (which is currently all of them as
the more modern and solid /dev/fd method is still broken, see #67),
process substitutions could leave background processes hanging in
these two scenarios:
1. If the parent process exits without opening a pipe to the child
process forked by the process substitution. The fifo_check()
function in xec.c, which is periodically called to check if the
parent process still exists while waiting for it to open the
FIFO, verified the parent process's existence by checking if the
PPID had reverted to 1, the traditional PID of init. However,
POSIX specifies that the PPID can revert to any implementation-
defined system process in that case. So this breaks on certain
systems, causing unused process substitutions to hang around
forever as they never detect that the parent disappeared.
The fix is to save the current PID before forking and having the
child check if the PPID has changed from that saved PID.
2. If command invoked from the main shell is passed a process
substitution, but terminates without opening the pipe to the
process substitution. In that case, the parent process never
disappears in the first place, because the parent process is the
main shell. So the same infinite wait occurs in unused process
substitutions, even after correcting problem 1.
The fix is to remember all FIFOs created for any number of
process substitutions passed to a single command, and unlink any
remaining FIFOs as they represent unused command substitutions.
Unlinking them FIFOs causes sh_open() in the child to fail with
ENOENT on the next periodic check, which can easily be handled.
Fixing these problems causes the FIFO method to act identically to
the /dev/fd method, which is good for compatibility. Even when #67
is fixed this will still be important, as ksh also runs on systems
that do not have /dev/fd (such as AIX, HP-UX, and QNX), so will
fall back to using FIFOs.
--- Fix problem 1 ---
src/cmd/ksh93/sh/xec.c:
- Add new static fifo_save_ppid variable.
- sh_exec(): If a FIFO is defined, save the current PID in
fifo_save_ppid for the forked child to use.
- fifo_check(): Compare PPID against the saved value instead of 1.
--- Fix problem 2 ---
To keep things simple I'm abusing the name-value pair routines used
for variables for this purpose. The overhead is negligible. A more
elegant solution is possible but would involve adding more code.
src/cmd/ksh93/include/defs.h: _SH_PRIVATE:
- Define new sh.fifo_tree pointer to a new FIFO cleanup tree.
src/cmd/ksh93/sh/args.c: sh_argprocsubs():
- After launching a process substitution in the background,
add the FIFO to the cleanup list before freeing it.
src/cmd/ksh93/sh/xec.c:
- Add fifo_cleanup() that unlinks all FIFOs in the cleanup list and
clears/closes the list. They should only still exist if the
command never used them, however, just run 'unlink' and don't
check for existence first as that would only add overhead.
- sh_exec():
* Call fifo_cleanup() on finishing all simple commands (when
setting $?) or when a special builtin fails.
* When forking, clear/close the cleanup list; we do not want
children doing duplicate cleanup, particularly as this can
interfere when using multiple process substitutions in one
command.
* Process substitution handling:
> Change FIFO check frequency from 500ms to 50ms.
Note that each check sends a signal that interrupts open(2),
causing sh_open() to reinvoke it. This causes sh_open() to
fail with ENOENT on the next check when the FIFO no longer
exists, so we do not need to add an additional check for
existence to fifo_check(). Unused process substitutions now
linger for a maximum of 50ms.
> Do not issue an error message if errno == ENOENT.
- sh_funct(): Process substitutions can be passed to functions as
well, and we do not want commands within the function to clean up
the FIFOs for the process substitutions passed to it from the
outside. The problem is solved by simply saving fifo_tree in a
local variable, setting it to null before running the function,
and cleaning it up before restoring the parent one at the end.
Since sh_funct() is called recursively for multiple-level
function calls, this correctly gives each function a locally
scoped fifo_tree.
--- Tests ---
src/cmd/ksh93/tests/io.sh:
- Add tests covering the failing scenarios.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit fixes two interrelated problems.
1. The -v unary test/[/[[ operator is documented to test if a
variable is set. However, it always returns true for variable
names with a numeric attribute, even if the variable has not
been given a value. Reproducer:
$ ksh -o nounset -c 'typeset -i n; [[ -v n ]] && echo $n'
ksh: n: parameter not set
That is clearly wrong; 'echo $n' should never be reached and the
error should not occur, and does not occur on mksh or bash.
2. Fixing the previous problem revealed serious breakage in short
integer type variables that was being masked. After applying
that fix and then executing 'typeset -si var=0':
- The conditional assignment expansions ${var=123} and
${var:=123} assigned 123 to var, even though it was set to 0.
- The expansions ${var+s} and ${var:+n} incorrectly acted as if
the variable was unset and empty, respectively.
- '[[ -v var ]]' and 'test -v var' incorrectly returned false.
The problems were caused by a different storage method for short
ints. Their values were stored directly in the 'union Value'
member of the Namval_t struct, instead of allocated on the stack
and referred to by a pointer, as regular integers and all other
types do. This inherently broke nv_isnull() as this leaves no
way to distinguish between a zero value and no value at all.
(I'm also pretty sure it's undefined behaviour in C to check for
a null pointer at the address where a short int is stored.)
The fix is to store short ints like other variables and refer
to them by pointers. The NV_INT16P combined bit mask already
existed for this, but nv_putval() did not yet support it.
src/cmd/ksh93/bltins/test.c: test_unop():
- Fix problem 1. For -v, only check nv_isnull() and do not check
for the NV_INTEGER attribute (which, by the way, is also used
for float variables by combining it with other bits).
See also 5aba0c72 where we recently fixed nv_isnull() to
work properly for all variable types including short ints.
src/cmd/ksh93/sh/name.c: nv_putval():
- Fix problem 2, part 1. Add support for NV_INT16P. The code is
simply copied and adapted from the code for regular integers, a
few lines further on. The regular NV_SHORT code is kept as this
is still used for some special variables like ${.sh.level}.
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Fix problem 2, part 2. Use NV_INT16P instead of NV_SHORT.
src/cmd/ksh93/tests/attributes.sh:
- Add set/unset/empty/nonempty tests for all numeric types.
src/cmd/ksh93/tests/bracket.sh,
src/cmd/ksh93/tests/comvar.sh:
- Update a couple of existing tests.
- Add test for [[ -v var ]] and [[ -n ${var+s} ]] on unset
and empty variables with many attributes.
src/cmd/ksh93/COMPATIBILITY:
- Add a note detailing the change to test -v.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Correct 'typeset -C' documentation. Variables declared as
compound are *not* initially unset, but initially have the empty
compound value. 'typeset' outputs them as:
typeset -C foo=()
and not:
typeset -C foo
and nv_isnull() is never true for them. This may or may not
technically be a bug. I don't think it's worth changing, but
it should at least be documented correctly.
These expansions are supposed to yield all variable names beginning
with the indicated prefix. This should include the variable name
that is identical to the prefix (as 'prefix' begins with 'prefix').
This bugfix is backported from the abandoned ksh 93v- beta, so AT&T
intended this change. It also makes ksh work like bash in this.
src/cmd/ksh93/sh/macro.c: varsub(): M_NAMESCAN:
- Check if the prefix itself exists. If so, start with that.
src/cmd/ksh93/tests/variables.sh:
- Add tests for these expansions.
src/cmd/ksh93/sh.1:
- Fix the incomplete documentation of these expansions.
src/cmd/ksh93/COMPATIBILITY:
- Note the change as it's potentially incompatible in corner cases.
Resolves: https://github.com/ksh93/ksh/issues/183
This commit fixes at least three bugs:
1. When issuing 'typeset -p' for unset variables typeset as short
integer, a value of 0 was incorrectly diplayed.
2. ${x=y} and ${x:=y} were still broken for short integer types
(re: 9f2389ed). ${x+set} and ${x:+nonempty} were also broken.
3. A memory fault could occur if typeset -l followed a -s option
with integers. Additonally, now the last -s/-l wins out as the
option to utilize instead of it always being short.
src/cmd/ksh93/include/name.h:
- Fix the nv_isnull() macro by removing the direct exclusion of
short integers from this set/unset test. This breaks few things
(only ${.sh.subshell} and ${.sh.level}, as far as we can tell)
while potentially correcting many aspects of short integer use
(at least bugs 1 and 2 above), as this macro is widely used.
- union Value: add new pid_t *pidp pointer member for PID values
(see further below).
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- To fix bug 3 above, unset the 'shortint' flag and NV_SHORT
attribute bit upon encountering the -l optiobn.
*** To fix ${.sh.subshell} to work with the new nv_isnull():
src/cmd/ksh93/sh/defs.h:
- Add new 'realsubshell' member to the shgd (aka shp->gd) struct
which will be the integer value for ${.sh.subshell}.
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/data/variables.c:
- Initialize SH_SUBSHELLNOD as a pointer to shgd->realsubshell
instead of using a short value (.s) directly. Using a pointer
allows nv_isnull() to return a positive for ${.sh.subshell} as
a non-null pointer is what it checks for.
- While we're at it, initialize PPIDNOD ($PPID) and SH_PIDNOD
(${.sh.pid}) using the new pdip union member, which is more
correct as they are values of type pid_t.
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Update the ${.sh.subshell} increases/decreases to refer to
shgd->realsubshell (a.k.a. shp->gd->realsubshell).
*** To fix ${.sh.level} after changing nv_isnull():
src/cmd/ksh93/sh/macro.c: varsub():
- Add a specific exception for SH_LEVLNOD to the nv_isnull() test,
so that ${.sh.level} is always considered to be set. Its handling
throughout the code is too complex/special for a simple fix, so
we have to special-case it, at least for now.
*** Regression test additions:
src/cmd/ksh93/tests/attributes.sh:
- Add in missing short integer tests and correct the one that
existed. The -si test now yields 'typeset -x -r -s -i foo'
instead of 'typeset -x -r -s -i foo=0' which brings it in line
with all the others.
- Add in some other -l attribute tests for floats. Note, -lX test
was not added as the size of long double is platform dependent.
src/cmd/ksh93/tests/variables.sh:
- Add tests for ${x=y} and ${x:=y} used on short int variables.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Symptoms of this bug below. These only seem to occur on Linux and
only if you replace your initial login shell by ksh using 'exec'.
1. An erroneous 'Interrupt' message is printed after stopping the
read builtin in a script. Reproducer:
$ exec arch/*/bin/ksh
$ cat ./reproducer.sh
#!/bin/sh
read foo
$ ./reproducer.sh
^C$ <Enter>
[1] + Interrupt ../reproducer.sh
2. Ctrl+C fails to stop /bin/package make. Reproducer:
$ exec arch/*/bin/ksh
$ mv arch arch.old
$ bin/package make
# Press Ctrl+C multiple times
Analysis: In 41ebb55a, I made an error in changing job_init() to
work correctly on non-interactive shells. This line from before:
552| if(possible = (setpgid(0,job.mypgid)>=0) || errno==EPERM)
was changed to:
555| possible = (setpgid(0,job.mypgid) >= 0);
556| if(sh_isoption(SH_INTERACTIVE) && (possible || errno==EPERM))
That is wrong. Before, 'possible' was set to 1 (true) if setpgid()
either succeeded or failed with EPERM. After, it is only set to 1
if setpgid() succeeds. As a result, job control initialisation is
aborted later on upon a test for non-zero 'possible'.
src/cmd/ksh93/sh/jobs.c: job_init():
- Once again set possible to 1 even if setpgid() fails with EPERM.
Thanks to @JohnoKing for the bug report and reproducers.
Resolves: https://github.com/ksh93/ksh/issues/210
The fix for '.' and '..' in regular globbing broke '.' and '..' in
globstar. No globstar pattern that contains '.' or '..' as any
pathname component still matched. This commit fixes that.
This commit also makes symlink/** mostly work, which it never has
done in any ksh93 version. It is correct and expected that symlinks
found by patterns are not resolved, but symlinks were not resolved
even when specified as explicit non-pattern pathname components.
For example, /tmp/** breaks if /tmp is a symlink (e.g. on macOS),
which looks like a bug.
src/lib/libast/include/glob.h,
src/lib/libast/misc/glob.c: glob_dir():
- Make symlink/** work. we can check if the string pointed to by
pat is exactly equal to *. If so, we are doing regular globbing
for that particular pathname element, and it's okay to resolve
symlinks. If not (if it's **), we're doing globstar and we should
not be matching symlinks.
- Let's also introduce proper identification of symlinks (GLOB_SYM)
and not lump them in with other special files (GLOB_DEV).
- Fix the bug with literal '.' and '..' components in globstar
patterns. In preceding code, the matchdir pointer gets set to the
complete glob pattern if we're doing globstar for the current
pathname element, null if not. The pat pointer gets set to the
elements of the pattern that are still left to be processed;
already-done elements are trimmed from it by increasing the
pointer. So, to do the right thing, we need to make sure that '.'
or '..' is skipped if, and only if, it is the final element in
the pattern (i.e., if pat does not contain a slash) and is not
specified literally as '.' or '..', i.e., only if '.' or '..' was
actually resolved from a glob pattern. After this change,
'**/.*', '**/../.*', etc. do the right thing, showing all your
hidden files and directories without undesirable '.' and '..'
results; '.' and '..' are skipped as final elements, unless you
literally specify '**/.', '**/..', '**/foo/bar/..', etc.
src/cmd/ksh93/COMPATIBILITY:
- Note the symlink/** globstar change.
src/cmd/ksh93/sh.1:
- Try to document the current globstar behaviour more exhausively.
src/cmd/ksh93/tests/glob.sh:
- Add tests. Try to cover all the corner cases.
src/cmd/ksh93/tests/shtests:
- Since tests in glob.sh do not use err_exit, they were not
counted. Special-case glob.sh for counting the tests: count the
lines starting with a test_* function call.
Resolves: https://github.com/ksh93/ksh/issues/146
Analysis: When a syntax error occurs, the shell performs a
longjmp(3) back to exfile() in main.c on line 417:
415| if(jmpval)
416| {
417| Sfio_t *top;
418| sh_iorestore((void*)shp,0,jmpval);
419| hist_flush(shp->gd->hist_ptr);
420| sfsync(shp->outpool);
The first thing it does is restore the file descriptor state
(sh_iorestore), then it flushes the history file (hist_flush), then
it synchronises sfio's logical stream state with the physical
stream state using (sfsync).
However, the fix applied in e999f6b1 caused sh_iorestore() to sync
all sfio streams unconditionally. So this was done before
hist_flush(), which caused unpredictable behaviour, including
temporary and/or permanent history corruption, as this also synched
shp->outpool before hist_flush() had a chance to do its thing.
The fix is to only call sfsync() in sh_iorestore() if we're
actually about to call ftruncate(2), and not otherwise.
Moral of the story: bug fixes should be as specific as possible to
minimise the risk of side effects.
src/cmd/ksh93/sh/io.c: sh_iorestore():
- Only call sfsync() if we're about to truncate a file.
src/cmd/ksh93/tests/pty.sh:
- Add test.
Thanks to Marc Wilson for reporting the bug and to Johnothan King
for finding the commit that introduced it.
Resolves: https://github.com/ksh93/ksh/issues/209
Relevant: https://github.com/att/ast/issues/61
src/cmd/ksh93/edit/edit.c: ed_read():
- The loop that handles SIGWINCH assumes sfpkrd will return and
set errno to EINTR if ksh is sent SIGWINCH. This only occurs
when select(2) is used to wait for input, so tell sfpkrd to
use select if possible. This is only done if the last argument
given to sfpkrd is '2', which should avoid regressions.
src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Always use select if the last argument is 2. This allows
sfpkrd() to intercept SIGWINCH when necessary.
Fixes: https://github.com/ksh93/ksh/issues/202
These POSIX expansions first assign y to x if x is unset or empty,
respectively, and then they yield the value of x. This was not
working on any ksh93 version if x was typeset as numeric (integer
or float) but still unset, as in not assigned a value.
$ unset a; typeset -i a; printf '%q\n' "${a:=42}" "$a"
0
''
Expected output:
42
42
src/cmd/ksh93/sh/macro.c:
- Fix the test for set/unset variable. It was broken because it
only checked for the existence of the node, which exists after
'typeset', but did not check if a value had been assigned. This
additional check needs to be done with the nv_isnull() macro, but
only for expansions of the regular M_BRACE type. Special
expansions cannot have an unset state.
- As of commit 95294419, we know that an nv_optimize() call may be
needed before using nv_isnull() if the shell is compiled with
SHOPT_OPTIMIZE. Move the nv_optimize() call from that commit
forward to before the new check that calls nv_isnull(), and only
bother with it if the type is M_BRACE.
src/cmd/ksh93/tests/variables.sh:
- Add tests for this bug. Test float and integer, and also check
that ${a=b} and ${a:=b} correctly treat the value of 'b' as an
arithmetic expression of which the result is assigned to 'a' if
'a' was typeset as numeric.
src/cmd/ksh93/tests/attributes.sh,
src/cmd/ksh93/tests/comvar.sh,
src/cmd/ksh93/tests/nameref.sh,
src/cmd/ksh93/tests/types.sh:
- Fix a number of tests to report failures correctly.
Resolves: https://github.com/ksh93/ksh/issues/157
The old Bourne shell failed to check for closing quotes and command
substitution backticks when encountering end-of-file in a parser
context (such as a script). ksh93 implemented a hack for partial
compatibility with this bug, tolerating unbalanced quotes and
backticks in backtick command subsitutions, 'eval', and command
line invocation '-c' scripts only.
This hack became broken for backtick command substitutions in
fe20311f/350b52ea as a memory leak was fixed by adding a newline to
the stack at the end of the command substitution. That extra
newline becomes part of any string whose quotes are not properly
terminated, causing problems such as the one detailed here:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01889.html
$ touch abc
$ echo `ls "abc`
ls: abc
: not found
No other fix for the memory leak is known that doesn't cause other
problems. (The alternative fix detailed in the referenced mailing
list post causes a different corner-case regression.)
Besides, the hack has always caused other corner case bugs as well:
$ ksh -c '((i++'
Actual: ksh: i++(: not found
(If an external command 'i++(' existed, it would be run)
Expect: ksh: syntax error at line 1: `(' unmatched
$ ksh -c 'i=0; echo $((++i'
Actual: (empty line; the arithmetic expansion is ignored)
Expect: ksh: syntax error at line 1: `(' unmatched
$ ksh -c 'echo $(echo "hi)'
Actual: ksh: syntax error at line 1: `(' unmatched
Expect: ksh: syntax error at line 1: `"' unmatched
So, it's time to get rid of this hack. The old Bourne shell is
dead and buried. No other shell tries to support this breakage.
Tolerating syntax errors is just asking for strange side effects,
inconsistent states, and corner case bugs. We should not want to do
that. Old scripts that rely on this will just need to be fixed.
src/cmd/ksh93/sh/lex.c:
- struct lexdata: Remove 'char balance' member for remembering an
unbalanced quote or backtick.
- sh_lex(): Remove the back to remember and compensate for
unbalanced quotes/backticks that was executed only if we were
executing a script from a string, as opposed to a file.
src/cmd/ksh93/COMPATIBILITY:
- Note the change.
Resolves: https://github.com/ksh93/ksh/issues/199
This commit fixes an arbitrary command execution vulnerability in
array subscripts used within the arithmetic subsystem.
One of the possible reproducers is:
var='1$(echo INJECTION >&2)' ksh -c \
'typeset -A a; ((a[$var]++)); typeset -p a'
Output before this commit:
INJECTION
typeset -A a=([1]=1)
The 'echo' command has been surreptitiously executed from an
external environment variable.
Output after this commit:
typeset -A a=(['1$(echo INJECTION >&2)']=1)
The value is correctly used as an array subscript and nothing in it
is parsed or executed. This is as it should be, as ksh93 supports
arbitrary subscripts for associative arrays.
If we think about it logically, the C-style arithmetic subsystem
simply has no business messing around with shell expansions or
quoting at all, because those don't belong to it. Shell expansions
and quotes are properly resolved by the main shell language before
the arithmetic subsystem is even invoked. It is particularly
important to maintain that separation because the shell expansion
mechanism also executes command substitutions.
Yet, the arithmetic subsystem subjected array subscripts that
contain `$` (and only array subscripts -- how oddly specific) to
an additional level of expansion and quote resolution. For some
unfathomable reason, there are two lines of code doing specifically
this. The vulnerability is fixed by simply removing those.
Incredibly, variants of this vulnerability are shared by bash, mksh
and zsh. Instead of fixing it, it got listed in Bash Pitfalls!
http://mywiki.wooledge.org/BashPitfalls#y.3D.24.28.28_array.5B.24x.5D_.29.29
src/cmd/ksh93/sh/arith.c:
- scope(): Remove these two lines that implement the vulnerability.
if(strchr(sub,'$'))
sub = sh_mactrim(shp,sub,0);
- scope(), arith(): Remove the NV_SUBQUOTE flag from two
nv_endsubscript() calls. That flag causes the array subscript to
retain the current level of shell quoting. The shell quotes
everything as in "double quotes" before invoking the arithmetic
subsystem, and the bad sh_mactrim() call removed one level of
quoting. Since we're no longer doing that, this flag should no
longer be passed, or subscripts may get extra backslash escapes.
src/cmd/ksh93/include/name.h,
src/cmd/ksh93/sh/array.c:
- nv_endsubscript(): The NV_SUBQUOTE flag was only passed from
arith.c. Since it is now unused, remove it.
src/cmd/ksh93/tests/arith.sh:
- Tweak some tests: fix typos, report wrong values.
- Add 21 tests. Most are based on reproducers contributed by
@stephane-chazelas and @hyenias. They verify that this
vulnerability is gone and that no quoting bugs were introduced.
Resolves: https://github.com/ksh93/ksh/issues/152
Corrected the size of attribute(s) being overwritten with 0 when
'readonly' or 'typeset -r' was applied to an existing variable. Since
one cannot set any attributes with the 'readonly' command, its function
call to setall() needs to be adjusted to acquire the current size from
the old size or existing size of the variable. A plain 'typeset -r' is
the same as 'readonly' in that it needs to load the old size as its
current size for use in the subsequent to call to nv_newattr().
src/cmd/ksh93/bltins/typeset.c: setall():
- Both 'readonly' and 'typeset -r' end up calling setall(). setall()
has full visibility into all user supplied values and existing
values that are needed to differentiate whereas name.c newattr()
acquires combined state flags.
- Added a conditional check if the readonly flag was requested by
user then meets the criteria of having present size of 0, cannot
be a numeric nor binary string, and is void of presence of any of
the justified string attributes.
- -L/R/Z justified string attributes if not given a value default
to a size of 0 which means to autosize. A binary string can have
a fixed field size, e.g. -bZ. The present of any of the -L/R/Z
attribules means that current size is valid and should be used
even if it is zero.
src/cmd/ksh93/tests/attributes.sh:
- Added various tests to capture and reiterate that 'readonly' should
be equivalent to 'typeset -r' and applying them should not alter the
previous existing size unless additional attributes are set along
with typeset command.
The value of the ${.sh.fun} variable, which is supposed to contain
the name of the function currently being executed, leaks out of the
DEBUG trap if it executes a function. Reproducer:
$ fn() { echo "executing the function"; }
$ trap fn DEBUG
$ trap - DEBUG
executing the function
$ echo ${.sh.fun}
fn
${.sh.fun} should be empty outside the function.
Annalysis:
The sh_debug() function in xec.c, which executes the DEBUG trap
action, contains these lines, which are part of restoring the state
after running the trap action with sh_trap():
nv_putval(SH_PATHNAMENOD,shp->st.filename,NV_NOFREE);
nv_putval(SH_FUNNAMENOD,shp->st.funname,NV_NOFREE);
shp->st = savst;
First the SH_PATHNAMENOD (${.sh.file}) and SH_FUNNAMENOD
(${.sh.fun}) variables get restored from the values in the shell's
scoped information struct (shp->st), but that is done *before*
restoring the parent scope with 'shp->st = savst;'. It should be
done after. Fixing the order is sufficient to fix the bug.
However, I am not convinced that these nv_putval() calls are good
for anything at all. Setting, unsetting, restoring, etc. the
${.sh.fun} and ${.sh.file} variables is already being handled
perfectly well elsewhere in the code for executing functions and
sourcing dot scripts. The DEBUG trap is neither here nor there.
There's no reason for it to get involved with these variables.
I was unable to break anything after simply removing those two
lines. So I strongly suspect this is another case, out of many now,
where a bug in ksh93 is properly fixed by removing some code.
I couldn't get ${.sh.file} to leak similarly -- I think this is
because SH_PATHNAMENOD (and not SH_FUNNOD) is set explicitly in
exfile() in main.c, masking this incorrect restore. It is the only
place where SH_PATHNAMENOD and SH_FUNNOD are not both set.
src/cmd/ksh93/sh/xec.c:
- Remove these two spurious nv_putval() calls.
src/cmd/ksh93/tests/variables.sh:
- Add regression test for leaking ${.sh.fun}.
1. The editor accepted literal tabs without escaping in certain
cases, causing buggy and inconsistent completion behaviour.
https://github.com/ksh93/ksh/issues/71#issuecomment-656970959https://github.com/ksh93/ksh/issues/71#issuecomment-657216472
2. After completing a filename by choosing from a file completion
menu, the terminal cursor was placed one position too far to the
right, corrupting command line display. This happened with
multiline active.
https://github.com/ksh93/ksh/issues/71#issue-655093805
3. A completion menu was displayed if the file name to be completed
was at the point where the rest of it started with a number,
even if that part uniquely identified it so the menu had 1 item.
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00436.html
src/cmd/ksh93/edit/emacs.c:
- Cosmetic consistency: change two instances of cntl('[') to ESC.
- ed_emacsread(): Fix number 1 by refusing to continue into default
processing if a tab character was not used for tab completion.
Instead, beep and continue to the next read loop iteration. This
behaviour is consistent with most other shells, so I doubt there
will be objections. To enter a literal tab it's simple enough to
escape it with ^V (the 'stty lnext' character) or \.
- draw(): Fix number 2 by correcting an off-by-one error in the
ed_setcursor() call that updates the terminal's cursor display
in multiline mode. The 'old' and 'new' parameters need to have
identical values in this particular call to avoid the cursor
position being off by one to the right. This change makes it
match the corresponding ed_setcursor() call in vi.c. See below*
for details. Thanks to Lev Kujawski for the help in analysing.
src/cmd/ksh93/edit/completion.c: ed_expand():
- Fix number 3 by changing from '=' mode (menu-based completion) to
'\' mode (ordinary filename completion) if the menu would only
show one option, which was pointless and annoying. This never
happened in vi mode, so possibly the ed_expand() call in emacs.c
could have been improved instead. But I'm comfortable with fixing
it here and not in emacs.c, because this fixes it at a more
fundamental level, plus it's straightforward and obvious here.
Resolves: https://github.com/ksh93/ksh/issues/71
____
* Further details on bug number 2:
At https://github.com/ksh93/ksh/issues/71#issuecomment-786391565
Martijn Dekker wrote:
> I'm back to my original hypothesis that there is somehow an
> off-by-one error related to the ed_setcursor() call that gets
> executed when in multiline mode. I cannot confirm whether that
> off-by-one error is actually in the call itself, or occurs
> sometime earlier on one of the many possible occasions where
> ep->cursor is changed. But everything else appears to work
> correctly, so it's not unlikely that the problem is in the call
> itself.
>
> For reference, this is the original version of that call in
> emacs.c:
>
> ksh/src/cmd/ksh93/edit/emacs.c
> Lines 1556 to 1557 in df2b9bf
> if(ep->ed->e_multiline && option == REFRESH)
> ed_setcursor(ep->ed, ep->screen, ep->cursor-ep->screen, ep->ed->e_peol, -1);
>
> There is a corresponding call in the vi.c refresh() function
> (which does the same thing as draw() in emacs.c), where the third
> (old) and fourth (new) arguments are actually identical:
>
> ksh/src/cmd/ksh93/edit/vi.c
>
> Lines 2086 to 2087 in df2b9bf
> if(vp->ed->e_multiline && vp->ofirst_wind==INVALID)
> ed_setcursor(vp->ed, physical, last_phys+1, last_phys+1, -1);
>
> The expectation for this particular call is in fact that they
> should be identical, so that a delta of zero is calculated in
> that function. Delta not being zero is what causes the cursor to
> be positioned wrong.
>
> In vi.c, last_phys is a macro that is defined as editb.e_peol,
> and editb is a macro that is defined as (*vp->ed). Which means
> last_phys means vp->ed->e_peol, which is the same as
> ep->ed->e_peol in emacs.c. (These editors were originally
> separate programs by different authors, and I suppose this is how
> it shows. Korn didn't want to change all the variable names to
> integrate them, so made macros instead.)
>
> That leaves the question of why vi.c adds 1 to both last_phys
> a.k.a. e_peol arguments, and emacs.c uses e_peol for new without
> adding anything. Analysing the ed_setcursor() code could answer
> that question.
>
> So, this patch makes emacs.c do it the same way vi.c does. Let's
> make the third argument identical to the fourth. My brief testing
> shows the bug is fixed, and the regression tests yield no
> failures. This fix is also the most specific change possible, so
> there are few opportunities for side effects (I hope).
At https://github.com/ksh93/ksh/issues/71#issuecomment-786466652
Lev Kujawski wrote:
> I did a bit of research on this, and I think the fix to have the
> Emacs editing mode do the same as Vi is correct.
>
> From RELEASE:
> 08-05-01 In multiline edit mode, the refresh operation will now clear
> the remaining portion of the last line.
>
> Here's a fragment from the completion.c of the venerable but
> dated CDE DtKsh:
>
> else
> while (*com)
> {
> *out++ = ' ';
> out = strcopy(out,*com++);
> }
> *cur = (out-outbuff);
> /* restore rest of buffer */
> out = strcopy(out,stakptr(0));
> *eol = (out-outbuff);
>
> Noticeably missing is the code to add a space after file name
> completions. So, it seems plausible that if multiline editing
> mode was added beforehand,the ep->ed->p_eol !=
> ep->cursor-ep->screen case might never have occurred during
> testing.
>
> Setting the 'first' parameter to -1 seems to be a pretty explicit
> indicator that the author(s) intended the line clearing code to
> run, hence the entry in RELASE.
>
> The real issue is that if we update the cursor by calling
> ed_setcursor on line 1554 with old != new, the later call to
> setcursor on line 1583, here:
>
> I = (ncursor-nscreen) - ep->offset;
> setcursor(ep,i,0);
>
> will use outdated screen information to call setcursor, which,
> coincidentally, calls ed_setcursor.
In the emacs editor:
1. press ESC
2. change the size of your terminal window
and your screen is mysteriously cleared. (Until recent fixes, the
shell probably also crashed somewhere in the job control code.)
The cause is the way SIGWINCH is handled by ed_read() in edit.c.
For the emacs editor, it sends a Ctrl+L character to the input
buffer. The Ctrl+L command refreshes the command line. And it so
happens that ESC plus Ctrl+L is a command to clear the screen in
the emacs editor.
With the exeption of vi insert/command mode for which it uses a
shared flag, edit.c does not know the state of the editor, because
their data are internal to emacs.c and vi.c. So it doesn't know
whether you're in some mode that treats keyboard input specially.
Which means this way of dealing with SIGWINCH is fundamentally
misdesigned and is not worth fixing.
It gets sillier: in addition to sending keyboard commands, edit.c
was also communicating directly with emacs.c and vi.c via a flag,
e_nocrnl, which means 'please don't make Ctrl+L emit a linefeed'
(it normally refreshes on a new line but that is undesirable for
SIGWINCH). So there is already a hack that breaks the barrier
between edit.c and emacs.c/vi.c. Let's do that properly instead.
As of this commit, ed_read() does not send any fake keystrokes.
Instead, two extern functions, emacs_redraw() and vi_redraw(), are
defined for redrawing the command line. These are put in emacs.c
and vi.c so they have access to relevant static data and functions.
Then, instead of sending keyboard commands to the editor and
returning, ed_read() simply calls the redraw function for the
active editor, then continues and waits for input. Much cleaner.
src/cmd/ksh93/include/edit.h:
- Remove e_nocrnl flag from Edit_t struct.
- Define externs emacs_redraw() and vi_redraw(). Since Emacs_t and
Vi_t types are not known here, we have to declare void* pointers
and the functions will have to use typecasts.
src/cmd/ksh93/edit/edit.c:
- ed_read(): Call emacs_redraw() or vi_redraw() as per above.
- ed_getchar(): Remove comment about a nonexistent while loop.
src/cmd/ksh93/edit/emacs.c:
- Updates corresponding to removal of e_nocrnl flag.
- Add emacs_redraw(). This one is pretty simple. Refresh the
command line, then ed_flush() to update the cursor display.
src/cmd/ksh93/edit/vi.c:
- Updates corresponding to removal of e_nocrnl flag. Also remove a
similar internal 'nonewline' flag which is now equally redundant.
- Move the Ctrl+L handling code (minus writing the newline) into
the vi_redraw() function.
- Change two cases where vi set nonewline and sent Ctrl+L to itself
into simple vi_redraw() calls.
- Add vi_redraw(). This is more complicated as it incorporates the
previous Ctrl+L code. It needs an added refresh() call with a
check whether we're currently in command or insert mode, as those
use different refresh methods. Luckily edit.c already maintains
an *e_vi_insert flag in ed_getchar() that we can use. Since vi's
refresh() already calls ed_flush(), we don't need to add that.
ksh crashed in various different and operating system-dependent
ways when attempting to create or apply justification strings
using typeset -L/-R/-Z, especially if large sizes are used.
The crashes had two immediate causes:
- In nv_newattr(), when applying justification attributes, a buffer
was allocated for the justified string that was exactly 8 bytes
longer than the original string. Any larger justification string
caused a buffer overflow (!!!).
- In nv_putval(), when applying existing attributes to a new value,
the corresponding memmove() either did not zero-terminate the
justified string (if the original string was longer than the
justified string) or could read memory past the original string
(if the original string was shorter than the justified string).
Both scenarios can cause a crash.
This commit fixes other minor issues as well, such as a mysterious
8 extra bytes allocated by several malloc/realloc calls. This may
have been some naive attempt to paper over the above bugs. It seems
no one can make any other kind of sense of it.
A readjustment bug with zero-filling was also fixed.
src/cmd/ksh93/sh/name.c:
- nv_putval():
. Get rid of the magical +8 bytes for malloc and realloc. Just
allocate one extra byte for the terminating zero.
. Fix the memmove operation to use strncpy instead, so that
buffer overflows are avoided in both scenarios described above.
Also make it conditional upon a size adjustment actually
happening (i.e. if 'dot' is nonzero).
. Mild refactoring: combine two 'if(sp)' blocks into one;
declare variables only used there locally for legibility.
- nv_newattr():
* Replace the fatally broken "let's allocate string length + 8
bytes no matter the size of the adjustment" routine with a new
one based on work by @hyenias (see comments in #142). It is
efficient with memory, taking into account numeric types,
growing strings, and shrinking strings.
* Fix zero-filling in readjustment after changing the initial
size of a -Z attribute. If the number was zero, all zeros were
still skipped, leaving an empty string.
Thanks to @hyenias for originally identifying this breakage and
laying the groundwork for fixing nv_newattr(), and to @lijog for
the crash analysis that revealed the key to the nv_putval() fix.
Resolves: https://github.com/ksh93/ksh/issues/142
Resolves: https://github.com/ksh93/ksh/issues/181
So now we know what that faulty check for shp->indebug in sh_trap()
was meant to do: it was meant to pass down the trap handler's exit
status, via sh_debug(), down to sh_exec() (xec.c) so that it could
then skip the execution of the next command if the trap's exit
status is 2, as documented in the manual page. As of d00b4b39, exit
status 2 was not passed down, so this stopped working.
This commit reinstates that functionality, but without the exit
status bug in command substitutions caused by the old way.
src/cmd/ksh93/sh/fault.c: sh_trap():
- Save the trap's exit status before restoring the parent
envionment's exit status. Make this saved exit status the return
value of the function. (This does not break anything, AFAICT; the
majority of sh_trap() calls ignore the return value, and the few
that don't ignore it seem to expect it to return exactly this.)
src/cmd/ksh93/sh/xec.c: sh_exec():
- The sh_trap() fix has one side effect: whereas the exit status of
a skipped command was always 2 (as per the trap handler), now it
is always 0, because it gets reset in sh_exec() but no command is
executed. That is probably not a desirable change in behaviour,
so let's fix that here instead: set sh.exitval to 2 when skipping
commands.
src/cmd/ksh93/sh.1:
- Document that ${.sh.command} shell-quotes its arguments for use
by 'eval' and such. This fact was not documented anywhere, AFAIK.
src/cmd/ksh93/shell.3:
- Document that $? (exit status) is made local to trap handlers.
- Document that sh_trap() returns the trap handler's exit status.
src/cmd/ksh93/tests/basic.sh:
- Add test for this bug.
- Add a missing test for the exit status 255 functionality (if a
DEBUG trap handler yields this exit status and we're executing a
function or dot script, a return is triggered).
Fixes: https://github.com/ksh93/ksh/issues/187
This commit fixes a bug in the 'read' built-in: it did not properly
skip over multibyte characters. The bug never affects UTF-8 locales
because all UTF-8 bytes have the high-order bit set. But Shift-JIS
characters may include a byte corresponding to the ASCII backslash
character, which cauased buggy behaviour when using 'read' without
the '-r' option that disables backslash escape processing.
It also makes the regression tests compatible with Shift-JIS
locales. They failed with syntax errors.
src/cmd/ksh93/bltins/read.c:
- Use the multibyte macros when skipping over word characters.
Based on a patch from the old ast-developers mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01848.html
src/cmd/ksh93/include/defs.h:
- Be a bit smarter about causing the compiler to optimise out
multibyte code when SHOPT_MULTIBYTE is disabled. See the updated
comment for details.
src/cmd/ksh93/tests/locale.sh:
- Put all the supported locales in an array for future tests.
- Add test for the 'read' bug. Include it in a loop that tests
64 SHIFT-JIS character combinations. Only one fails on old ksh:
the one where the final byte corresponds to the ASCII backslash.
It doesn't hurt to test all the others anyway.
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/quoting2.sh:
- Fix syntax errors that occurred in SHIFT-JIS locales as the
parser was processing literal UTF-8 characters. Not executing
that code is not enough; we need to make sure it never gets
parsed as well. This is done by wrapping the commands containing
literal UTF-8 strings in an 'eval' command as a single-quoted
operand.
.github/workflows/ci.yml:
- Run the tests in the ja_JP.SJIS locale instead of ja_JP.UTF-8.
UTF-8 is already covered by the nl_NL.UTF-8 test run; that should
be good enough.
A ${ shared-state command substitution; } (internally called
subshare) is documented to share its state with the parent shell
environment, so all changes made within the command substitution
survive outside of it. However, when it is run within a
virtual/non-forked subshell, variables that are not already local
to that subshell will leak out of it into the grandparent state.
Reproducer:
$ ksh -c '( v=${ bug=BAD; } ); echo "$bug"'
BAD
If the variable pre-exists in the subshell, the bug does not occur:
$ ksh -c '( bug=BAD1; v=${ bug=BAD2; } ); echo "$bug"'
(empty line, as expected)
The problem is that the sh_assignok() function, which is
responsible for variable scoping in virtual subshells, does not
ever bother to create a virtual subshell scope for a subshare.
That is an error if a subshare's parent (or higher-up ancestor)
environment is a virtual subshell, because a scope needs to be
created in that parent environment if none exists.
To make this bugfix possible, first we need to get something out of
the way. nv_restore() temporarily sets the subshell's pointer to
the preesnt working directory, shpwd, to null. This causes
sh_assignok() to assume that the subshell is a subshare (because
subshares don't store their own PWD) and refuse to create a scope.
However, nv_restore() sets it to null for a different purpose: to
temporarily disable scoping for *all* virtual subshells, making
restoring possible. This is a good illustration of why it's often
not a good idea to use the same variable for unrelated purposes.
src/cmd/ksh93/sh/subshell.c:
- Add a global static subshell_noscope flag variable to replace the
misuse of sh.shpwd described above.
- sh_assignok():
. Check subshell_noscope instead of shpwd to see if scope
creation is disabled. This makes it possible to distinguish
between restoring scope and handling subshares.
. If the current environment is a subshare that is in a virtual
subshell, create a scope in the parent subshell. This is done
by temporarily making the parent virtual subshell the current
subshell (by setting the global subshell_data pointer to it)
and calling sh_assignok() again, recursively.
- nv_restore(): To disable subshell scope creation while restoring,
set subshell_noscope instead of saving and unsetting sh.shpwd.
src/cmd/ksh93/tests/subshell.sh:
- Add tests. I like tests. Tests are good.
Fixes: https://github.com/ksh93/ksh/issues/143
This commit fixes the following:
1. Emacs mode ignores --nobackslashctrl (re: 24598fed) when in
reverse search.
2. When entering more than one backslash, emacs reverse search mode
deletes multiple backslashes after pressing backspace once.
Reproducer:
$ set --emacs --nobackslashctrl
$ <Ctrl+R> \\\\<Backspace>
3. Except when in reverse search, the backslash fails to escape a
subsequent interrupt character (^C). Reproducer:
$ set --emacs --backslashctrl
$ teststring \<Ctrl+C>
src/cmd/ksh93/edit/emacs.c:
- Disable escaping backslashes in emacs reverse search if
'nobackslashctrl' is enabled.
- Fix the buggy behavior of backslashes in emacs reverse
search by processing backslashes in a loop.
src/cmd/ksh93/tests/pty.sh:
- Add regression tests.
src/cmd/ksh93/sh.1:
- Fix a minor documentation error (^C is the usual interrupt
character, not ^?).
Co-authored-by: Martijn Dekker <martijn@inlv.org>
'case x in esac' should be syntactically correct, but was an error:
$ ksh -c 'case x in esac'
ksh: syntax error at line 1: `case' unmatched
Inserting a newline was a workaround:
$ ksh -c $'case x in\nesac'
(no output)
The problem was that the 'esac' reserved word was not being
recognised if it immediately followed the 'in' reserved word.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Do not turn off recognition of reserved words after 'in' if we're
in a 'case' construct; only do this for 'for' and 'select'.
src/cmd/ksh93/tests/case.sh:
- Add seven regression test for correct recognition of 'esac'.
Only two failed on ksh93. The rest is to catch future bugs.
Fixes: https://github.com/ksh93/ksh/issues/177
This commit fixes the functionality of Alt+D and Alt+H in emacs mode.
These keyboard shortcuts are intended to work on whole words, but
after commit 13c3fb21 their functionality was reduced to deleting only
singular letters:
$ Test word <Alt+H> # This should delete 'word', not just 'd'.
$ Foo <Alt+B> <Alt+D> # This should delete 'Foo', not just 'F'.
Man page entries for reference:
M-d Delete current word.
M-^H (Meta-backspace) Delete previous word.
M-h Delete previous word.
src/cmd/ksh93/edit/emacs.c:
- 'count' cannot be overridden when handling Alt+D or Alt+H,
so add the total number of repetitions to count (the number of
repetitions can't be negative).
- If 'count' is a negative number, set it to one before adding the
number of repetitions.
The following emacs editor 'feature' kept making me want to go back
to bash. I forget a backslash escape in a command somewhere. So I
go back to insert it. I type the \, then want to go forward. My
right arrow key, instead of moving the cursor, then replaces my
backslash with garbage. Why? The backslash escapes the following
control character at the editor level and inserts it literally.
The vi editor has a variant of this which is much less harmful. It
only works in insert mode and the backslash only escapes the next
kill or erase character.
In both editors, this feature is completely redundant with the
'stty lnext' character which is ^V by default -- and works better
as well because it also escapes ^C, ^J (linefeed) and ^M (Return).
[In fact, you could even issue 'stty lnext \\' and get a much more
consistent version of this feature on any shell. You have to type
two backslashes to enter one, but it won't kill your cursor keys.]
If it were up to me alone, I'd simply remove this misfeature from
both editors. However, it is long-standing documented behaviour.
It's in the 1995 book. Plus, POSIX specifies the vi variant of it.
So, this adds a shell option instead. It was quite trivial to do.
Now I can 'set --nobackslashctrl' in my ~/.kshrc. What a relief!
Note: To keep .kshrc compatibile with older ksh versions, use:
command set --nobackslashctrl 2>/dev/null
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/data/options.c:
- Add new SH_NOBACKSLCTRL/"nobackslashctrl" long-form option. The
"no" prefix shows it to the user as "backslashctrl" which is on
by default. This avoids unexpectedly changing historic behaviour.
src/cmd/ksh93/edit/emacs.c: ed_emacsread(),
src/cmd/ksh93/edit/vi.c: getline():
- Only set the flag for special backslash handling if
SH_NOBACKSLCTRL is off.
src/cmd/ksh93/sh.1,
src/cmd/ksh93/data/builtins.c:
- Document the new option (as "backslashctrl", on by default).
Other minor tweaks:
src/cmd/ksh93/edit/edit.c:
- ed_setup(): Add fallback #error if no tput method is set. This
should never be triggered; it's to catch future editing mistakes.
- escape(): cntl('\t') is nonsense as '\t' is already a control
character, so change this to just '\t'.
- xcommands(): Let's enable the ^X^D command for debugging
information on non-release builds.
src/cmd/ksh93/features/cmds:
- The tput feature tests assumed a functioning terminal in $TERM.
However, for all we know we might be compiling with no tty and
TERM=dumb. The tput commands won't work then. So set TERM=ansi
to use a standard default.
This fixes the following:
1. 'set --posix' now works as an equivalent of 'set -o posix'.
2. The posix option turns off braceexpand and turns on letoctal.
Any attempt to override that in a single command such as 'set -o
posix +o letoctal' was quietly ignored. This now works as long
as the overriding option follows the posix option in the command.
3. The --default option to 'set' now stops the 'posix' option, if
set or unset in the same 'set' command, from changing other
options. This allows the command output by 'set +o' to correctly
restore the current options.
src/cmd/ksh93/data/builtins.c:
- To make 'set --posix' work, we must explicitly list it in
sh_set[] as a supported option so that AST optget(3) recognises
it and won't override it with its own default --posix option,
which converts the optget(3) string to at POSIX getopt(3) string.
This means it will appear as a separate entry in --man output,
whether we want it to or not. So we might as well use it as an
example to document how --optionname == -o optionname, replacing
the original documentation that was part of the '-o' description.
src/cmd/ksh93/sh/args.c: sh_argopts():
- Add handling for explitit --posix option in data/builtins.c.
- Move SH_POSIX syncing SH_BRACEEXPAND and SH_LETOCTAL from
sh_applyopts() into the option parsing loop here. This fixes
the bug that letoctal was ignored in 'set -o posix +o letoctal'.
- Remember if --default was used in a flag, and do not sync options
with SH_POSIX if the flag is set. This makes 'set +o' work.
src/cmd/ksh93/include/argnod.h,
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/sh/args.c: sh_printopts():
- Do not potentially translate the 'on' and 'off' labels in 'set
-o' output. No other shell does, and some scripts parse these.
src/cmd/ksh93/sh/init.c: sh_init():
- Turn on SH_LETOCTAL early along with SH_POSIX if the shell was
invoked as sh; this makes 'sh -o' and 'sh +o' show expected
options (not that anyone does this, but correctness is good).
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- The state flags were in defs.h and most (but not all) of the
shell options were in shell.h. Gather all the shell state and
option flag definitions into one place in shell.h for clarity.
- Remove unused SH_NOPROFILE and SH_XARGS option flags.
src/cmd/ksh93/tests/options.sh:
- Add tests for these bugs.
src/lib/libast/misc/optget.c: styles[]:
- Edit default optget(3) option self-documentation for clarity.
Several changed files:
- Some SHOPT_PFSH fixes to avoid compiling dead code.
If I haven't missed anything, this should make the non-interactive
aspects of job control in scripts work as expected, except for the
"<command unknown>" issue in the output of 'bg', 'fg' and 'jobs'
(which is not such a high priority as those commands are really
designed for interactive use).
Plus, I believe I now finally understand what these three are for:
* The job.jobcontrol variable is set to nonzero by job_init() in
jobs.c if, and only if, the shell is interactive *and* managed to
get control of the terminal. Therefore, any changing of terminal
settings (tcsetpgrp(3), tty_set()) should only be done if
job.jobcontrol is nonzero. This commit changes several checks for
sh_isoption(SH_INTERACTIVE) to checks for job.jobcontrol for
better consistency with this.
* The state flag, sh_isstate(SH_MONITOR), determines whether the
bits of job control that are relevant for both scripts and
interactive shells are active, which is mostly making sure that a
background job gets its own process group (setpgid(3)).
* The shell option, sh_isoption(SH_MONITOR), is just that. When the
user turns it on or off, the state flag is synched with it. It
should usually not be directly checked for, as the state may be
temporarily turned off without turning off the option.
Prior discussion:
https://www.mail-archive.com/austin-group-l@opengroup.org/msg06456.html
src/cmd/ksh93/bltins/typeset.c, src/cmd/ksh93/sh/args.c:
- Move synching the SH_MONITOR state flag with the SH_MONITOR
shell option from b_set() (the 'set' builtin) to sh_applyopts()
which is indirectly called from b_set() and is also used when
parsing the shell invocation command line. This ensures -m is
properly enabled in both scenarios.
src/cmd/ksh93/sh/jobs.c:
- job_init(): Do not refuse to initialise job control on
non-interactive shells. Instead, skip everything that should only
be done on interactive shells (i.e., everything to do with the
terminal). This function is now even more of a mess than it was
before, so refactoring may be desirabe at some point.
- job_close(), job_set(), job_reset(), job_wait(): Do not reset the
terminal process group (tcsetpgrp()) if job.jobcontrol isn't on.
src/cmd/ksh93/sh/xec.c:
- sh_exec(): TFORK: For SIGINT handling, check the SH_MONITOR
state flag, not the shell option.
- sh_exec(): TFORK: Do not turn off the SH_MONITOR state flag in
forked children. The non-interactive part of job control should
stay active. Instead, turn off the SH_INTERACTIVE state flag so
we don't get interactive shell behaviour (i.e. job control noise
on the terminal) in forked subshells.
- _sh_fork(), sh_ntfork(): Do not reset the terminal process group
(tcsetpgrp()) if job.jobcontrol isn't on. Do not turn off the
SH_MONITOR state flag in forked children.
src/cmd/ksh93/sh/subshell.c: sh_subfork():
- Do not turn off the monitor option and state in forked subshells.
The non-interactive part of job control should stay active.
src/cmd/ksh93/bltins/misc.c: b_bg():
- Check isstate(SH_MONITOR) instead of sh_isoption(SH_MONITOR) &&
job.jobcontrol before throwing a 'no job control' error.
This fixes a minor bug: fg, bg and disown could quietly fail.
src/cmd/ksh93/tests/jobs.sh:
- Add tests for 'fg' with job control IDs (%%, %1) in scripts.
- Add test checking that a background job launched from a subsell
with job control enabled correctly becomes the leader of its own
process group.
Makes progress on: https://github.com/ksh93/ksh/issues/119
Another longstanding whopper of a bug in basic ksh93 functionality:
run a ${ shared-state; } command substitution twice and job control
promptly loses track of all your running jobs. New jobs are tracked
again until you run another two shared-state command substitutions.
This is in at least 93t+, 93u-, 93u+, 93v- and ksh2020.
$ sleep 300 &
[1] 56883
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi1; }
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi2; }
$ jobs # Nothing!
$ fg
ksh: fg: no such job
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- The current environment number shp->curenv (a.k.a. sh.curenv) was
not being restored if the virtual subshell we're leaving is of
the shared-state command substitution variety as it was wrongly
considered to be part of the environment that didn't need
restoring. This caused it to be out of sync with shp->jobenv
(a.k.a. sh.jobenv) which did get restored from savedcurenv.
Restore both from savedcurenv at the same time for any subshell.
(How these numbers are used exactly remains to be discovered.)
src/cmd/ksh93/tests/jobs.sh:
- Added, with a test for this bug to start it off. There is no
other test script where job control fits, and a lot more related
fixes are anticipated: https://github.com/ksh93/ksh/issues/119
This is some nonsense: redirections that store a file descriptor
greater than 9 in a variable, like {var}<&2 and the like, stopped
working if brace expansion was turned off. '{var}' is not a brace
expansion as it doesn't contain ',' or '..'; something like 'echo
{var}' is always output unexpanded. And redirections and brace
expansion are two completely unrelated things. It wasn't documented
that these redirections require the -B/braceexpand option, either.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Remove incorrect check for braceexpand option before processing
redirections of this form.
src/cmd/ksh93/COMPATIBILITY:
- Insert a brief item mentioning this.
src/cmd/ksh93/sh.1:
- Correction: these redirections do not yield a file descriptor >
10, but > 9, a.k.a. >= 10.
- Add a brief example showing how these redirections can be used.
src/cmd/ksh93/tests/io.sh:
- Add a quick regression test.
This commit corrects how shortint was being applied to various
possible typeset variables in error. The short integer option
modifier 'typeset -s' should only be able to be applied if the
the variable is also an integer. Several issues were resolved
with this fix:
- 'typeset -s': created a short integer having an invalid base
of zero. 'typeset -s foo' created 'typeset -s -i 0 foo=0' and
now will result in an empty string.
- 'typeset -sL': previously resulted in a segmentation fault.
The following are the various incorrect 'typeset' instances
that have been fixed:
$ 'export foo; typeset -s foo; readonly foo; typeset -p foo'
(before) typeset -x -r -s -i 0 foo=0
( after) typeset -x -r foo
$ 'typeset -sL foo=1*2; typeset -p foo'
(before) Segmentation fault (core dumped)
( after) typeset -L 3 foo='1*2'
$ 'typeset -sR foo=1*2; typeset -p foo'
(before) typeset -s -i foo=2
( after) typeset -R 3 foo='1*2'
$ 'typeset -sZ foo=1*2; typeset -p foo'
(before) typeset -F 0 foo=2
( after) typeset -Z 3 -R 3 foo='1*2'
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Add conditional check within the 's' option to only
apply NV_SHORT as well as remove any NV_LONG flag
if NV_INTEGER flag was set.
- Relocate shortint conditional logic to the 'i' option.
src/cmd/ksh93/tests/attributes.sh:
- Adjust regression tests for '-s' and add '-si' check.
This fixes part of https://github.com/ksh93/ksh/issues/87:
Scalar arrays (-a) and associative arrays (-A) of a type created by
'enum' did not consistently block values not specified by the enum
type, yielding corrupted results.
An expansion of type "${array[@]}" yielded random numbers instead
of values for associative arrays of a type created by 'enum'.
This does not yet fix another problem: ${array[@]} does not yield
all values for associative enum arrays.
src/cmd/ksh93/bltins/enum.c: put_enum():
- Always throw an error if the value is not in the list of possible
values for an enum type. Remove incorrect check for the NV_NOFREE
flag. Whatever that was meant to accomplish, I've no idea.
src/cmd/ksh93/sh/array.c: nv_arraysettype():
- Instead of sh_eval()ing a shell assignment, use nv_putval()
directly. Also use the stack (see src/lib/libast/man/stk.3)
instead of malloc to save the value; it's faster and will be
auto-freed at some point. This shortens the function and makes it
faster by not entering into a whole new shell context -- which
also fixes another problem: the error message from put_enum()
didn't cause the shell to exit for indexed enum arrays.
src/cmd/ksh93/sh/name.c: nv_setlist():
- Apply a patch from David Korn that correctly sets the data type
for associative arrays, fixing the ${array[@]} expansion yielding
random numbers. Thanks to @JohnoKing for the pointer.
https://github.com/ksh93/ksh/issues/87#issuecomment-662613887https://www.mail-archive.com/ast-developers@lists.research.att.com/msg00697.html
src/cmd/ksh93/tests/enum.sh:
- Add tests checking that invalid values are correctly blocked for
indexed and associative arrays of an enum type.
Makes progress on: https://github.com/ksh93/ksh/issues/87
This commit fixes 'command -x' to adapt to OS limitations with
regards to data alignment in the arguments list. A feature test is
added that detects if the OS aligns the argument on 32-bit or
64-bit boundaries or not at all, allowing 'command -x' to avoid
E2BIG errors while maximising efficiency.
Also, as of now, 'command -x' is a way to bypass built-ins and
run/query an external command. Built-ins do not limit the length of
their argument list, so '-x' never made sense to use for them. And
because '-x' hangs on Linux and macOS on every ksh93 release
version to date (see acf84e96), few use it, so there is little
reason not to make this change.
Finally, this fixes a longstanding bug that caused the minimum exit
status of 'command -x' to be 1 if a command with many arguments was
divided into several command invocations. This is done by replacing
broken flaggery with a new SH_XARG state flag bit.
src/cmd/ksh93/features/externs:
- Add new C feature test detecting byte alignment in args list.
The test writes a #define ARG_ALIGN_BYTES with the amount of
bytes the OS aligns arguments to, or zero for no alignment.
src/cmd/ksh93/include/defs.h:
- Add new SH_XARG state bit indicating 'command -x' is active.
src/cmd/ksh93/sh/path.c: path_xargs():
- Leave extra 2k in the args buffer instead of 1k, just to be sure;
some commands add large environment variables these days.
- Fix a bug in subtracting the length of existing arguments and
environment variables. 'size -= strlen(cp)-1;' subtracts one less
than the size of cp, which makes no sense; what is necessary is
to substract the length plus one to account for the terminating
zero byte, i.e.: 'size -= strlen(cp)+1'.
- Use the ARG_ALIGN_BYTES feature test result to match the OS's
data alignment requirements.
- path_spawn(): E2BIG: Change to checking SH_XARG state bit.
src/cmd/ksh93/bltins/whence.c: b_command():
- Allow combining -x with -p, -v and -V with the expected results
by setting P_FLAG to act like 'whence -p'. E.g., as of now,
command -xv printf
is equivalent to
whence -p printf
but note that 'whence' has no equivalent of 'command -pvx printf'
which searches $(getconf PATH) for a command.
- When -x will run a command, now set the new SH_XARG state flag.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Change to using the new SH_XARG state bit.
- Skip the check for built-ins if SH_XARG is active, so that
'command -x' now always runs an external command.
src/lib/libcmd/date.c, src/lib/libcmd/uname.c:
- These path-bound builtins sometimes need to run the external
system command by the same name, but they did that by hardcoding
an unportable direct path. Now that 'command -x' runs an external
command, change this to using 'command -px' to guarantee using
the known-good external system utility in the default PATH.
- In date.c, fix the format string passed to 'command -px date'
when setting the date; it was only compatible with BSD systems.
Use the POSIX variant on non-BSD systems.
This commit resolves the following incorrect variable assignments:
$ unset a; typeset -uF a=2; typeset -p a
typeset -X a=0x1.0000000000p+1
$ unset a; typeset -Fu a=2; typeset -p a
typeset -X a=0x1.0000000000p+1
$ unset a; typeset -ulF a=2; typeset -p a
typeset -l -X a=0x1.0000000000p+1
$ unset a; typeset -Ful a=2; typeset -p a
typeset -l -X a=0x1.0000000000p+1
$ unset a; typeset -Eu a=2; typeset -p a
typeset -E -X a=2
$ unset a; typeset -Eul a=2; typeset -p a
typeset -l -E -X a=2
src/cmd/ksh93/bltins/typeset.c:
- If the unsigned option (-u) was provided in conjunction with a
floating point (-F) then due to a flag collision with NV_UNSIGN
and NV_HEXFLOAT both having the value of NV_LTOU caused the
floating point to become a hexadecimal floating point (-X) in
error. Also, if a -E option flag was followed with a -u option
then the resulting variable would be both a scientific notation
and a hexadecimal floating point at the same time.
src/cmd/ksh93/tests/attributes.sh:
- Add regression tests.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
An unquoted variable expansion evaluated in a DEBUG trap action
caused IFS field splitting to be deactivated in code executed after
the trap action. Thanks to Koichi Nakashima for the reproducer:
| v=''
| trap ': $v' DEBUG
| A="a b c"
| set -- $A
| printf '%s\n' "$@"
|
| Expected
|
| a
| b
| c
|
| Actual
|
| a b c
src/cmd/ksh93/sh/fault.c: sh_trap():
- Remove incorrect save/restore of sh.ifstable, the internal state
table for field splitting. This reverts three lines added in ksh
93t+ 2009-11-30. Analysis: As an expansion is split into fields
(macro.c, lines 2367-2471), sh.ifstable is modified. If that
happens within a DEBUG trap, any modifications in ifstable are
undone by the restoring memccpy, leaving an inconsistent state.
src/cmd/ksh93/COMPATIBILITY:
- Document the DEBUG trap fixes, particularly the incorrect
inheritance by subshells and functions that some scripts may now
rely on because this bug is so longstanding. (re: 2a835a2d)
src/cmd/ksh93/tests/basic.sh:
- Add relevant tests.
Resolves: https://github.com/ksh93/ksh/issues/155
TODO: add a -T (-o functrace) option as in bash, which should allow
subshells and ksh-style functions to inherit DEBUG traps.
P.S.: The very handy multishell repo allows us to use 'git blame'
to trace the origin of the recently fixed DEBUG trap bugs.
The off-by-one error causing various bugs, reverted in 2a835a2d,
was introduced in ksh 93t 2008-07-25:
https://github.com/multishell/ksh93/commit/8e947ccf
(fault.c, line 321)
The incorrect check causing the exit status bug, reverted in
d00b4b39, was introduced in ksh 93t 2008-11-04:
https://github.com/multishell/ksh93/commit/b1ade268
(fault.c, line 459)
The ifstable save/restore causing the field splitting bug, reverted
in this commit, was introduced in ksh 93t+ 2009-11-30:
https://github.com/multishell/ksh93/commit/53d9f009
(fault.c, lines 440, 444, 482)
So all the bugs reported in #155 were fixed by simply reverting
these specific changes. I think that they are some experiments that
the developers simply forgot to remove. I've suspected such a thing
multiple times before. ksh93 was developed by researchers who were
genius innovators, but incredibly sloppy maintainers.
Turns out the previous commit also fixed the bug that disables the
DEBUG trap if a redirection is used in a DEBUG trap action -- in
other words, that's the same bug.
src/cmd/ksh93/tests/basic.sh:
- Add test from the reproducer in the bug report.
Makes progress on: https://github.com/ksh93/ksh/issues/155
This trap failed to be restored correctly when being trapped in
a subshell, causing corruption or a crash when restoring the
parent shell environment's trap upon leaving the subshell.
Thanks to Koichi Nakashima for the report and reproducer.
src/cmd/ksh93/sh/fault.c: sh_sigreset():
- Fix an off-by-one error in the loop that restores the
pseudosignal traps.
src/cmd/ksh93/tests/basic.sh:
- Test overwriting the main shell trap in a subshell for all
pseudosignals.
Makes progress on: https://github.com/ksh93/ksh/issues/155
The build system is adapted to make SHOPT_* compile-time options
editable without nmake. We can now easily change ksh's compile-time
options by editing src/cmd/ksh93/SHOPT.sh. The bin/package script
is adapted to turn these into compile flags. This resolves the most
important drawback of not using nmake.
Also, mamake now has support for indented Mam (Make Abstract
Machine) code. Only one type of block (make...done) is supported in
Mamfiles, so they are easy to indent automatically. A script to
(re)do this is included.
Since nmake is not going to be restored (it has too many problems
that no one is interested in fixing), this at least makes mamake
significantly easier to work with.
The Makefiles are deleted. They may still be handy for reference to
understand the Mamfiles, but they haven't actually matched the
Mamfiles for a while -- and you can still look in the git history.
Deleting them requires some adaptations to bin/package and mamake.c
because, even though they do not use those files, they still looked
for them to decide whether to build code in a directory.
Finally, this commit incorporates some #pragmas for clang to
suppress annoying warnings about the coding style used in this
historic code base. (gcc does not complain so much.)
src/cmd/ksh93/SHOPT.sh:
- Added.
bin/package, src/cmd/INIT/package.sh:
- cd into our own directory in case we were run from another dir.
- $makefiles: only look for Mamfiles.
- Add ksh compile-options via KSH_SHOPTFLAGS. Include SHOPT.sh.
- make_recurse(): Do not write a missing Makefile.
- finalize environment: Look for Mamfiles instead of Makefiles.
src/cmd/INIT/mamake.c:
- Tell clang to suppress annoying warnings about coding style.
- Update version string and self-documentation.
- input(): Add support for indented Mam code by skipping initial
whitespace on each input line.
- files[]: Instead of looking for various of Makefiles to decide
where to build, only look for Mamfiles.
src/Makefile, src/cmd/INIT/Makefile, src/cmd/Makefile,
src/cmd/builtin/Makefile, src/cmd/ksh93/Makefile, src/lib/Makefile,
src/lib/libast/Makefile, src/lib/libcmd/Makefile,
src/lib/libdll/Makefile, src/lib/libsum/Makefile:
- Removed.
src/Mamfile, src/cmd/INIT/Mamfile, src/cmd/Mamfile,
src/cmd/builtin/Mamfile, src/cmd/ksh93/Mamfile, src/lib/Mamfile,
src/lib/libast/Mamfile, src/lib/libcmd/Mamfile,
src/lib/libdll/Mamfile, src/lib/libsum/Mamfile:
- Indent the code with tabs.
- In ksh93/Mamfile, add ${KSH_SHOPT_FLAGS} to every $CC command.
- In ksh93/Mamfile, add "prev SHOPT.sh" for every *.o file
so they are rebuilt whenever SHOPT.sh changes.
bin/Mamfile_indent:
- Added, in case someone wants to re-indent a Mamfile.
src/cmd/INIT/proto.c, src/cmd/INIT/ratz.c, src/cmd/INIT/release.c,
src/lib/libast/features/common, src/lib/libast/include/ast.h:
- Tell clang to suppress annoying warnings about coding style that
it disapproves of (mainly concerning the use of parentheses).
src/cmd/INIT/cc.darwin, src/cmd/INIT/cc.freebsd,
src/cmd/INIT/cc.openbsd:
- Remove now-redundant clang warning suppression flags.
Resolves: https://github.com/ksh93/ksh/issues/60
This adds informative error messages if incompatible options are
given. It also documents the exclusive -m, -n and -T options on
separate usage lines, as was already done with -f. The usage
message for incompatible options now looks something like this:
| $ ksh -c 'typeset -L10 -F -f -i foo'
| ksh: typeset: -i/-F/-E/-X cannot be used with -L/-R/-Z
| ksh: typeset: -f cannot be used with other options
| Usage: typeset [-bflmnprstuxACHS] [-a[type]] [-i[base]] [-E[n]]
| [-F[n]] [-L[n]] [-M[mapping]] [-R[n]] [-X[n]]
| [-h string] [-T[tname]] [-Z[n]] [name[=value]...]
| Or: typeset -f [name...]
| Or: typeset -m [name=name...]
| Or: typeset -n [name=name...]
| Or: typeset -T [tname[=(type definition)]...]
| Help: typeset [ --help | --man ] 2>&1
(see also the previous commit, e21a053e)
Unfortunately the first "Usage" line has some redundancies with the
"Or:" lines showing separate usages. It doesn't seem to be possible
to avoid this; it's a flaw in how libast generates everything
(usage, help, manual) from one huge getopt(3) string. I still think
the three added "Or:" lines are an improvement as it wasn't
previously shown that these options need to be used on their own.
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Instead of only showing a generic usage message, add an
informative error message if incompatible options were given.
- Conflicting options detection was failing because NV_LJUST and
NV_EXPNOTE have the same bitmask value. Use a new 'isadjust'
flag for -L/-R/-Z to remember if one of these was set.
- Detect conflict between -L/-R/-Z and a float option, not just -i.
src/cmd/ksh93/include/name.h, src/cmd/ksh93/data/msg.c:
- Add the two new error messages for incompatible options.
src/cmd/ksh93/data/builtins.c: sh_opttypeset[]:
- Add a space after 'float' in in "[+float?\btypeset -lE\b]" as
this makes 'float' appear on its own line, improving formatting.
- Show -m, -n, -T on separate usage lines like -f, as none of these
can be combined with other options.
- Remove "cannot be combined with other options" from -m and -n
descriptions, as that should now be clear from the separate usage
lines -- and even if not, the error message is now informative.
src/cmd/ksh93/sh.1, src/cmd/ksh93/COMPATIBILITY:
- Update.
src/cmd/ksh93/tests/types.sh:
- Remove obsolete test: 'typeset -RF' is no longer accepted.
(It crashed in 93u+, so this is not an incompatibility...)
Resolves: https://github.com/ksh93/ksh/issues/48
This fixes the following:
trap ':' DEBUG
r=$(exit 123)
echo $? # Expected 123, but actually 0.
Thanks to Koichi Nakashima for the report and reproducer.
src/cmd/ksh93/sh/fault.c: sh_trap():
- Restore the saved current exit status (exitval) for all traps.
Do not except the DEBUG trap from doing that. I've no idea why
this exception was made, but it's not correct.
src/cmd/ksh93/tests/basic.sh:
- Add tests.
Makes progress on: https://github.com/ksh93/ksh/issues/155
This commit also further mitigates the problems with restoring an
inaccessible or nonexistent PWD on exiting a virtual subshell.
Harald van Dijk writes:
> On a build of ksh with -fsanitize=undefined to help diagnose
> problems:
>
> $ mkdir deleted
> $ cd deleted
> $ rmdir ../deleted
> $ ksh -c '(cd /; (cd /)); :'
> /home/harald/ksh/src/cmd/ksh93/sh/subshell.c:561:22: runtime
> error: null pointer passed as argument 1, which is declared to
> never be null
> Segmentation fault (core dumped)
>
> Note that it segfaults the same with default compilation flags,
> but it does not print out the useful extra message. The code
> assumes that pwd is non-null and passes it to strcmp without
> checking, but it will be null if the current directory cannot be
> determined, for instance because it has been deleted.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Avoid the null pointer dereference reported above.
src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Fork a virtual subshell even on systems with fchdir(2) if the
present working directory tests as inaccessible on invoking 'cd';
it may no longer exist and fchdir would fail to get a handle.
(For the test we have to opendir(3) the full path to the PWD and
not ".", as the latter may succeed even if the PWD is gone.)
src/cmd/ksh93/data/builtins.c:
- Update 'cd' version string.
Fixes: https://github.com/ksh93/ksh/issues/153
Related: https://github.com/ksh93/ksh/issues/141
The forking fix implemented in 102868f8 and 9d428f8f, which stops
the main shell's hash table from being cleared if PATH is changed
in a subshell, can cause a significant performance penalty for
certain scripts that do something like
( PATH=... command foo )
in a subshell, especially if done repeatedly. This is because the
hash table is cleared (and hence a subshell forks) even for
temporary PATH assignments preceding commands.
It also just plain doesn't work. For instance:
$ hash -r; (ls) >/dev/null; hash
ls=/bin/ls
Simply running an external command in a subshell caches the path in
the hash table that is shared with a main shell. To remedy this, we
would have to fork the subshell before forking any external
command. And that would be an unacceptable performance regression.
Virtual subshells do not need to fork when changing PATH if they
get their own hash tables. This commit adds these. The code for
alias subshell trees (which was removed in ec888867 because they
were broken and unneeded) provided the beginning of a template for
their implementation.
src/cmd/ksh93/sh/subshell.c:
- struct subshell: Add strack pointer to subshell hash table.
- Add sh_subtracktree(): return pointer to subshell hash table.
- sh_subfuntree(): Refactor a bit for legibility.
- sh_subshell(): Add code for cleaning up subshell hash table.
src/cmd/ksh93/sh/name.c:
- nv_putval(): Remove code to fork a subshell upon resetting PATH.
- nv_rehash(): When in a subshell, invalidate a hash table entry
for a subshell by creating the subshell scope if needed, then
giving that entry the NV_NOALIAS attribute to invalidate it.
src/cmd/ksh93/sh/path.c: path_search():
- To set a tracked alias/hash table entry, use sh_subtracktree()
and pass the HASH_NOSCOPE flag to nv_search() so that any new
entries are added to the current subshell table (if any) and do
not influence any parent scopes.
src/cmd/ksh93/bltins/typeset.c: b_alias():
- b_alias(): For hash table entries, use sh_subtracktree() instead
of forking a subshell. Keep forking for normal aliases.
- setall(): To set a tracked alias/hash table entry, pass the
HASH_NOSCOPE flag to nv_search() so that any new entries are
added to the current subshell table (if any) and do not influence
any parent scopes.
src/cmd/ksh93/sh/init.c: put_restricted():
- Update code for clearing the hash table (when changing $PATH) to
use sh_subtracktree().
src/cmd/ksh93/bltins/cd_pwd.c:
- When invalidating path name bindings to relative paths, use the
subshell hash tree if applicable by calling sh_subtracktree().
- rehash(): Call nv_rehash() instead of _nv_unset()ting the hash
table entry; this is needed to work correctly in subshells.
src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for various PATH-related operations in the main
shell and in a virtual subshell.
- Several pre-existing memory leaks are exposed by the new tests
(I've confirmed these in 93u+). The tests are disabled and marked
TODO for now, as these bugs have not yet been fixed.
src/cmd/ksh93/tests/subshell.sh:
- Update.
Resolves: https://github.com/ksh93/ksh/issues/66
The SHOPT_2DMATCH code block in sh_setmatch() modifies the 'ap'
pointer, which is initialised as nv_arrayptr(SH_MATCHNOD). This
caused a (rarely occurring) segfault in the following line near the
end of the function:
ap->nelem -= x;
as this line assumed that 'ap' still had the initial value.
src/cmd/ksh93/sh/init.c: sh_setmatch():
- On init, save ap in ap_save and use ap_save instead of ap where
it should be pointing to SH_MATCHNOD. This also allows removing
two redundant nv_arrayptr(SH_MATCHNOD) calls, slightly increasing
the efficiency of this function.
@stephane-chazelas writes:
> Per POSIX[*], cd should skip the $CDPATH processing if the first
> component of the directory given to cd is . or ...
>
> Yet, with ksh93u+m 2021-01-03 at least, while that's OK with ..,
> it's not with . with or without the posix option:
>
> $ CDPATH=/ ./ksh -o posix -c 'cd -P ./etc && pwd'
> /etc
> /etc
>
> It seems to be a regression introduced with ksh93u+ as I can't
> reproduce it with ksh93u or any version prior to that. I can also
> reproduce in u+, v- and the ksh2020 from the Ubuntu 20.04
> package.
src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Skip $CDPATH processing not only if the path is absolute, but
also if the initial path component is '.' or '..' (in the latter
case the $CDPATH processing was done but appeared to be a no-op).
src/cmd/ksh93/tests/builtins.sh:
- Add regression test.
[*] https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/utilities/cd.html
Fixes: https://github.com/ksh93/ksh/issues/151
