The commit that backported read -a did not add a case label for it
to read.c. This was under the assumption that AST optget(3) would
always convert -a to -A. However, that was only done for first use.
The cause is the short-form options caching mechanism in optget(3).
On the first run, the pre-caching result would be returned, but the
equivalent option (-a) would be cached as if it is its own option,
so on the second and subsequent runs, optget returned 'a' instead
of 'A'. This only happens if no long-form equivalent is present.
Reproducer:
$ read -A foo <<< 'foo bar baz'
$ unset foo
$ read -a foo <<< 'foo bar baz'
$ echo ${foo[0]}
foo bar baz
Expected: foo
src/lib/libast/misc/optget.c,
src/lib/libast/misc/optlib.h:
- [by Martijn Dekker] Implement caching for short-option
equivalents. If a short-form equivalent is found, instead of
caching it as a separate option, cache the equivalent in a new
equiv[] array. Check this when reading the cache and substitute
the main option for the equivalent if one is cached.
src/lib/libcmd/cp.c:
- Fix cp -r/cp -R symlink handling. The -r and -R options sometimes
ignored -P, -L and -H.
- The -r and -R options no longer follow symlinks by default.
src/cmd/ksh93/bltins/whence.c,
src/lib/libcmd/*.c:
- Remove case labels that are redundant now that the optget(3)
caching bug is fixed.
src/cmd/ksh93/tests/libcmd.sh:
- Added. This is the new script for the /opt/ast/bin path-bound
built-ins from libcmd. Other relevant tests are moved into here.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
The referenced commit made -b/--notify much better at notifying
multiple terminating background jobs, but it still occasionally
misses one if multiple jobs terminate all at once. This race
condition will hopefully be worked out at some point, but it's a
low-priority problem as the notification will appear at the next
prompt in any case, and we still have many worse bugs to fix.
But the associated pty regression test is failing fairly often on
the github runners in the meantime, so this disables it for now.
The ASan crash in basic.sh when sourcing multiple files is caused by
a bug that is similar to the crash fixed in f24040ee. This is the
trace for the regression test crash (note that in order to see the
trace, the 2>/dev/null redirect must be disabled):
==1899388==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150000005b0 at pc 0x55a5e3f9432a bp 0x7ffeb91ea110 sp 0x7ffeb91ea100
WRITE of size 8 at 0x6150000005b0 thread T0
#0 0x55a5e3f94329 in funct /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:967
#1 0x55a5e3f96f77 in item /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:1349
#2 0x55a5e3f90c9f in term /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:642
#3 0x55a5e3f90ac1 in list /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:613
#4 0x55a5e3f90845 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:561
#5 0x55a5e3f909e0 in sh_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:586
#6 0x55a5e3f8fd5e in sh_parse /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/parse.c:438
#7 0x55a5e3fc43c1 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:635
#8 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318
#9 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254
#10 0x55a5e3fd01d4 in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1932
#11 0x55a5e3fc4544 in sh_eval /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:651
#12 0x55a5e4012172 in b_dot_cmd /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/bltins/misc.c:318
#13 0x55a5e3fca3cb in sh_exec /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/xec.c:1254
#14 0x55a5e3ecc1cd in exfile /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:604
#15 0x55a5e3ec9e7f in sh_main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/main.c:369
#16 0x55a5e3ec801d in main /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/pmain.c:41
#17 0x7f637b4db2cf (/usr/lib/libc.so.6+0x232cf)
#18 0x7f637b4db389 in __libc_start_main (/usr/lib/libc.so.6+0x23389)
#19 0x55a5e3ec7f24 in _start ../sysdeps/x86_64/start.S:115
Code in question:
8d57369b0c/src/cmd/ksh93/sh/parse.c (L963-L968)
To avoid any more similar crashes, all of the fixes introduced
in 69d37d5e that set slp->slptr to null have been improved with the
fix in f24040ee.
This macro expansion in lex.c may assign -1 to n if EOF is reached:
1178: fcgetc(n);
As a result, n may be -1 when this code is reached:
1190: if(sh_isoption(SH_BRACEEXPAND) && c==LBRACE && !assignment
&& state[n]!=S_BREAK
'state[n]' is a buffer overflow if n==-1.
src/cmd/ksh93/sh/lex.c: sh_lex(): case S_BRACE:
- Apart from the buffer overflow, if n<=0, none of the code
following fcget(n) does anything until 'break' on line 1199 is
reached. So, if fcget(n) yields <=0, just break. This allows some
code simplification.
Progresses: https://github.com/ksh93/ksh/issues/518
The referenced commit introduced the NIL (NULL) assignment in:
stakdelete(slpold->slptr);
slpold->slptr = NIL(Stak_t*);
First the stack is closed/freed with stakdelete() a.k.a.
stkclose(), then its pointer is reset. Looks correct, right?
Wrong: slpold may itself be in the allocated region that
slpold->slptr points to. That's because we're dealing with a linked
list of stacks, in which a pointer on each stack points to the next
stack. So there are scenarios in which, after the stakdelete()
call, dereferencing slpold is a use after free.
Most systems quietly tolerate this use after free. But, according
to @JohnoKing's testing, this bug was causing 23 crashes in the
regression tests after compiling ksh with AddressSanitizer enabled.
src/cmd/ksh93/sh/parse.c: sh_funstaks():
- Save the value of slpold->slptr and reset that pointer before
calling stakdelete() a.k.a. stkclose().
Resolves: https://github.com/ksh93/ksh/issues/517
This reverts commit 42fc5c4c0d.
It somehow caused the following reproducer to fail. Reason unknown.
typeset -i x
function x.set { :; }
x[0]=0
unset x
typeset -p x
Expected output: none
Actual output:
typeset -a -i x=()
The 'x' variable fails to be unset.
With the .get and .getn discipline functions instead of .set, the
above reproducer still fails even after the revert, but that always
has failed, back to at least 93u+ 2012-08-01.
This is yet more evidence that discipline functions are a mess.
But a proper cleanup will require a lot of time and very thorough
testing, so it will have to wait until some future release.
src/cmd/ksh93/tests/variables.sh:
- In addition to the revert, add the above reproducer as a
regression test for the .set, .unset and .append disciplines.
The dependency rule 'prev libshell.a archive' did not actually
cause shcomp to be rebuilt when something in libast.a changed.
Replace by 'prev ksh' to ensure shcomp is rebuilt/relinked whenever
ksh is.
Commit 4ca578bd accidentally left in half of an if statement that was
originally just a use of the SFMTXBEGIN2 macro (which is a no-op without
vt_threaded). As a result, the head builtin's -s flag broke in one of
the ksh2020 regression tests. Reproducer:
# Note that head must be enabled in src/cmd/ksh93/data/builtins.c
builtin head || exit 1
cat > "/tmp/file1" <<EOF
This is line 1 in file1
This is line 2 in file1
This is line 3 in file1
This is line 4 in file1
This is line 5 in file1
This is line 6 in file1
This is line 7 in file1
This is line 8 in file1
This is line 9 in file1
This is line 10 in file1
This is line 11 in file1
This is line 12 in file1
EOF
got=$(head -s 5 -c 18 "/tmp/file1")
exp=$'is line 1 in file1'
[[ "$got" = "$exp" ]] || print "'head -s' failed" "(expected $(printf %q "$exp"), got $(printf %q "$got"))"
Code change that caused this bug (note that since the if statement
wasn't completely removed, it now guards the for loop despite the
newline):
if(fw)
- SFMTXBEGIN2(fw, (Sfoff_t)0);
for(n_move = 0; n != 0; )
I noticed this since I'm making a separate commit to backport more of
the ksh2020 regression tests.
src/lib/libast/sfio/sfmove.c:
- Remove the incomplete if statement to fix the -s flag.
src/cmd/ksh93/tests/b_head.sh:
- Backport the ksh2020 regression tests for the head builtin.
Reproducer: on an interactive shell with the -H option on,
$ v=foo
$ print ${#v}
does not print anything (but should print "3").
The 'print' line also is not added to the history.
This bug was exposed by commit 41ee12a5 which enabled the history
comment character by default, setting it to '#' as on bash. When it
was disabled by default, this bug was rarely exposed.
The problem happens here:
src/cmd/ksh93/edit/hexpand.c
203: if(hc[2] && *cp == hc[2]) /* history comment designator, skip rest of line */
204: {
205: stakputc(*cp++);
206: stakputs(cp);
207: DONE();
208: }
The DONE() macro sets the HIST_ERROR bit flag:
src/cmd/ksh93/edit/hexpand.c
45: #define DONE() {flag |= HIST_ERROR; cp = 0; stakseek(0); goto done;}
For the history comment character that is clearly wrong, as no
error has occurred.
There is another problem. The documentation I added for history
expansion states this bit, which is based on bash's behaviour:
If a word on a command line begins with the history comment
character #, history expansion is ignored for the rest of that
line.
With an expansion like ${#v}, the word does not begin with # so
history expansion should not have parsed that as a comment
character. The intention was to act like bash.
src/cmd/ksh93/edit/hexpand.c:
- Split the DONE() macro into DONE and ERROROUT of which only the
latter sets the HIST_ERROR bit flag. Change usage accordingly.
Thix fixes the first problem.
- Don't make these new macros function-style macros (with ()) as
they end in a goto, so that's a bit misleading.
- Add is_wordboundary() which makes a best-effort attempt to
determine if the character following the specified character is
considered to start a new word by shell grammar rules. Word
boundary characters are whitespace and: |&;()`<>
- Only recognise the history comment character if is_wordbounary()
returns true for the previous character. This fixes the second
problem.
Thanks to @jghub for the bug report.
Resolves: https://github.com/ksh93/ksh/issues/513
This release fixes the interactive shell crashing when one of the
predefined aliases (currently 'history' and 'r') is redefined,
whether from a profile/kshrc script or manually. This crash
occurred in two scenarios:
1. when redefining and then unsetting a predefined alias;
2. when redefining a predefined alias and then executing a shell
script that does not begin with a #! path.
Both are fixed now.
Reproducer 1:
$ alias r
r='hist -s'
$ alias r=foo
$ unalias r
ksh(10127,0x10d6c35c0) malloc: *** error for object 0x7ffdcd404468: pointer being freed was not allocated
ksh(10127,0x10d6c35c0) malloc: *** set a breakpoint in malloc_error_break to debug
Abort
The crash happens as unall() (typeset.c) calls nv_delete() (name.c)
which tries to free a node pointer that was not directly allocated.
Reproducer 2:
$ ENV=/./dev/null ksh
$ echo : >script
$ chmod +x script
$ alias r=foo
$ ./script
ksh(10193,0x10c8505c0) malloc: *** error for object 0x7fa136c04468: pointer being freed was not allocated
ksh(10193,0x10c8505c0) malloc: *** set a breakpoint in malloc_error_break to debug
Abort
This crash happens for the same reason, but in another location,
namely in sh_reinit() (init.c) as it is freeing up the alias table
before executing a script that does not start with a #! path.
This is a serious bug because it is not uncommon for .kshrc or
.profile scripts to (re)define an alias called 'r'.
Analysis:
These crashes happen because the incorrectly freed node pointer is
part of a larger block of nodes initialised by sh_inittree() in
init.c. That function allocates all the nodes for a table (see
data/{aliases,builtins,variables}.c) in a contiguous block
addressable by numeric index (see builtins.h and variables.h for
how that is used).
So, while the value of the alias is correctly marked NV_NOFREE and
is not freed, that attribute does not apply to the node pointer
itself, which also is not freeable. Thus, if the value is replaced
by a freeable one, the node pointer is incorrectly freed upon
unaliasing it, and the shell crashes.
The simplest fix is to allocate each predefined alias node
individually, like any other alias -- because, in fact, we do not
need the predefined alias nodes to be in a contiguous addressable
block; there is nothing that specifically addresses these aliases.
src/cmd/ksh93/sh/main.c: sh_main():
- Instead of calling sh_inittree(), use a dedicated loop to
allocate each predefined alias node individually, making each
node invidually freeable. The value does remain non-freeable,
but the NV_NOFREE attribute correctly takes care of that.
src/cmd/ksh93/bltins/typeset.c:
- Get rid of the incomplete and now unnecessary workarounds in
unall() and sh_reinit().
Thanks to @jghub and @JohnoKing for finding and reporting the bug.
Discussion: https://github.com/ksh93/ksh/discussions/503#discussioncomment-3337172
As of 2022-06-18, ksh 93u+m is not capable of being used as /bin/sh
while building GNU binutils. The execution of some of its build
system's dot scripts is incorrectly aborted as an external 'sed'
command is execve(2)'d without forking. This means that incorrect
exec optimization was happening.
Unfortunately I have not been able to derive a minimal reproducer
of the problem yet because the GNU binutils build scripts are very
complex. Pending further research, the optimisation is reverted.
Even if a way to make it work is found, it will not be reintroduced
to the 1.0 branch.
Thanks to @atheik for finding the problem and identifying the
commit that introduced it.
Resolves: https://github.com/ksh93/ksh/issues/507
'bin/package install' breaks on pure POSIX shells because I used
the regex negator ^ instead of the glob pattern negator ! in a glob
pattern bracket pattern. Oops.
If the -xc99 or -std=c99 flag is passed to Solaris Studio cc, the
build fails with a syntax error on the 'noreturn' keyword. That
keyword was introduced in the C11 standard; C99 does not have it.
The features/common test decides that we can use the 'noreturn'
optimization if the <stdnoreturn.h> header is present on the
system, but that is not correct; standards flags may disable it.
src/lib/libast/features/common:
- Remove two unused tests for extern and void*, all part of C90
which we now require (re: a1f5c992).
- Add test that checks for 'noreturn' by compiling a test program.
- Use that test's result to decide whether to define 'noreturn' as
empty or not.
_ _ ___ _____ ___ ___ ___
| | _____| |__ / _ \___ / _ _ _ _ __ ___ / / | / _ \ / _ \
| |/ / __| '_ \ | (_) ||_ \| | | |_| |_| '_ ` _ \ / /| || | | | | | |
| <\__ \ | | | \__, |__) | |_| |_ _| | | | | |/ / | || |_| | |_| |
|_|\_\___/_| |_| /_/____/ \__,_| |_| |_| |_| |_/_/ |_(_)___(_)___/
It may have taken exactly a decade, but here we are... a proper new
ksh release. :) Many thanks to all contributors for their hard work!
Compared to an unpatched 93u+, this release has roughly a thousand
bugs fixed. It incorporates a fair number of enhancements as well.
Not all known bugs have been worked out yet; see the TODO file. Let's
hope this release will rekindle interest and attract more bug hunters.
This commit also makes some very minor fixes in comments. Notable:
src/cmd/ksh93/sh/arith.c: sh_strnum():
- Update a security-related comment. As of b48e5b33, evaluating
untrusted arithmetic expressions from the environment should no
longer cause CVE-2019-14868. But let's keep disallowing it anyway.
Resolves: https://github.com/ksh93/ksh/issues/491
These are a few minor edits to the man page for readability and consistency:
* Add missing "in" to or else a fifo in a temporary directory
* Some paragraphs had missing spacing between them
* Some lists were missing spacing before the following paragraph
* Use bullet points to list the rksh restrictions
* Indent the list of characters in the "Quoting" section
This now allows easy installation via:
bin/package install DESTINATION_DIRECTORY [ COMMAND ... ]
Commands are installed in bin and share/man subdirectories.
If no command is specified, ksh and shcomp are assumed.
[This commit was previously reverted because it seemed to cause the
build to fail on Cygwin. But I just re-tested it, and it's fine. It
may be that my Cygwin installation at the time was defective.]
Something similar was previously done in 07cc71b8 from a Debian
patch, and eventually reverted; it redefined the ast atomic
functions asoincint() and asodecint() to be gcc-specific. This
imports the upstream version from the ksh 93v- beta instead.
This commit is based on an OpenSUSE patch:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-joblock.dif
src/cmd/ksh93/include/jobs.h:
- Replace job locking mechanism with the 93v- version which uses
the atomic libast functions asoincint(), asogetint() and
asodecint(). See: src/lib/libast/man/aso.3
src/cmd/ksh93/sh/jobs.c: job_subsave():
- Revert gcc optimiser bug workaround from c258a04f.
It should now be unnecessary.
There were still intermittent leak test failures on Debian.
They're clearly spurious as a different test fails each time.
My testing on a Debian 11.4.0 VM showed that it takes not a
doubling, but a quadrupling of the maximum number of test
iterations (from 16384==2**14 to 65536==2**16) for that system's
memory state to stabilise enough to avoid spurious test failurese.
Resolves: https://github.com/ksh93/ksh/issues/501
When converting from -Z to another attribute, if the value of the
-Z variable was empty (so -Z0 and an empty value), the loop that
skips initial zeros may read before the beginning of the buffer:
2968: if(nv_isattr(np,NV_ZFILL))
2969: {
2970: while(*sp=='0') sp++; /* skip initial zeros */
2971: if(!*sp) sp--; /* if number was 0, leave one zero */
2972: }
If the *sp value is empty (just a terminating zero byte), line 2970
does nothing, but line 2971 still decrases the pointer, to before
the beginning of the buffer.
The fix is to check for an initial zero before running that block.
Reproducer (fails intermittently, depending on garbage before *sp):
typeset -Z foo=
typeset -i foo
macOS doesn't like it if ast.h is included in vmmopen.c
(conflicting type declarations for brk() and sbrk() between that
file and the OS header), so only include ast.h when we're actually
using the the NoN stub macro from it.
Thanks to Mark Wilson for the build failure report.
Resolves: https://github.com/ksh93/ksh/issues/499
This commit:
1. Updates the COPYRIGHT file with all the contributors to ksh
93u+m since the reboot, based on the commit history, sorted by
number of commits in descending order. It also separates the
authors of backported contributions from direct contributors.
2. Adds missing contributors of source each file to that file's
copyright header based on that file's commit history.
The script used to maintain this is now up at:
https://github.com/ksh93/ksh/wiki/update-copyright.sh
In addition, the original ast copyright notice is restored to
its EPL 1.0 version -- that is what AT&T Intellectual Property
originally licensed this code under, and we are the ones who
upgraded it to EPL 2.0 (as allowed by EPL 1.0) in 441dcc04.
The history should be accurate.
- Fixed a few minor typos and updated the list of tested systems in
src/cmd/ksh93/README.
- vmmopen.c: Include the ast.h header to fix a build error on Haiku
caused by the otherwise undefined NoN macro (re: 05f0c1b1).
In the documentation on the .unset discipline, this removes the
sentence: "The variable will not be unset unless it is unset
explicitly from within this discipline function."
Every ksh93 version published since at least 1995 unsets a variable
if 'unset' is run, regardless of whether any .unset discipline
function itself unsets the variable or not.
But all the documentation and books ever published (man page,
Bolsky, O'Reilly, IBM AIX docs) claim that an .unset discipline
needs to unset the variable again in order for it to be unset.
Bolsky (1995) p. 79, says:
> unset
> Called when the variable is unset. The variable will not be
> unset unless it is unset inside the dicipline function.
Other similar claims:
https://github.com/ksh93/ksh/issues/419#issuecomment-1199773432
So, ALL OF IT IS WRONG, believe it or not. Welcome to the
ksh93 twilight zone, where nothing is as it seems.
There are other bugs, see #419. But for now, other than correcting
the man page, I'm not touching this mess with a ten-foot pole.
The callback function for malloc(3) failure in the stk(3) routines
(which can be specified using stkinstall() and is set to nomemory()
in init.c) uses a size argument of type int. That's a type mismatch
with all the other size arguments and variables in stk(3) which use
size_t, an unsigned type that may be larger than int.
This is all quite inconsequential as nothing in our code base (or
in the complete old AT&T AST code base) actually uses that size for
anything, but it's still wrong, so this corrects the interface.
With this very minor API change, let's bump the libast API version
to 20220801, the date of the upcoming ksh 93u+m 1.0.0 release. :)
The ksh93/sh/init.c nomemory() function now reports the size that
could not be allocated, just because it can.
The bug is that "$*", and related expansions such as "${arr[*]}",
etc., do pattern matching if the first character of $IFS is a
wildcard. For example, the following:
IFS=*
set -- F ''
case BUGFREE in
BUG"$*") echo bug ;;
esac
outputs 'bug'. This bug can be reproduced in every other glob
pattern matching context as well, but not in pathname expansion.
src/cmd/ksh93/sh/macro.c: varsub():
- When joining fields into one for a "$*"-type expansion, check if
a glob pattern matching operation follows (mp->pattern is set).
If so, write a preceding backslash to escape the separator.
Resolves: https://github.com/ksh93/ksh/issues/489
Resolves: https://github.com/att/ast/issues/12
EPL 1.0 says, in section 7: "The Program (including Contributions)
may always be distributed subject to the version of the Agreement
under which it was received. In addition, after a new version of
the Agreement is published, Contributor may elect to distribute the
Program (including its Contributions) under the new version."
The Eclipse Foundation also encourage everyone to upgrade:
https://www.eclipse.org/legal/epl-2.0/faq.php#h.60mjudroo8e5https://www.eclipse.org/legal/epl-2.0/faq.php#h.tci84nlsqpgw
Unfortunately the new Secondary License option is not available to
us as we're not the original copyright holders and don't have the
legal power to add one. So, no GPL compatibility. Sorry.
Attempting to tab-complete the only member of a directory while in
vi mode switches to "control" under the following conditions:
1. pwd is outside of the directory
2. No part of the basename has been typed in (e.g., starting with
vim TEST/t will autocomplete to vim TEST/test, but starting
with vim TEST/ will not).
Also, trying to type with "input" or "replace" fails afterwards
until a new line has been generated (either with ctrl + c or the
j/k keys in control mode).
src/cmd/ksh93/edit/vi.c: textmod():
- The strange completion behavior in vi mode for single-member
directories appears to be at least partially due to completion
failing to use \ or * mode from outside of pwd. Checking for
e_nlist directly from vi.c — 0 unless in = mode — allows for
entering the else loop at line 2575 to reach APPEND mode.
Resolves: https://github.com/ksh93/ksh/issues/485
Since we now have sh.current_ppid, we might as well point a shell
variable to it, as the cost is nil.
Together, ${.sh.pid} and ${.sh.ppid} (updated when a virtual
subshell forks) form a logical counterpart pair to $$ and $PPID
(never updated in subshells).
This commit also adds a section to the manual page that hopefully
does away with the depressingly widespread subshell/child shell
confusion once and for all... :P
The undocumented .sh.dollar variable was a hack used to change the
value of $$ (main shell's PID); any value assigned to .sh.dollar
overrides the value of $$. This was used by libcoshell and by the
pre-fork(2) code to update the value of $$ in child processes
initialised by internally generated shell scripts. Both were
removed long ago, so we don't need this.
The Sfio code uses its own _has_multibyte macro and that was not
being disabled by AST_NOMULTIBYTE, so some of its multibyte code
was still getting compiled in. This is easily fixed in sfhdr.h.
But that exposed another issue: the Sfio stdio wrappers didn't
know about the _has_multibyte macro. So this adds the necessary
directives to keep their multibyte functions from compiling.
For $PPID and ${.sh.pid}, ksh has simply been assuming that pid_t
is a 32-bit integer. In POSIX, the size of pid_t is not specified
and operating systems are free to vary it. On any system where
sizeof(pid_t) != 4, these variables produce undefined results.
(Note that this is not a problem with $$ as it is converted to a
string value directly from pid_t in special() in macro.c.)
src/cmd/ksh93/features/options:
- Add feature test that produces NV_PID, a name-value attribute
macro that is set to NV_INTEGER|NV_SHORT (for int16_t),
NV_INTEGER (for int32_t) or NV_INTEGER|NV_LONG (for Sflong_t
a.k.a. intmax_t), depending on the size of pid_t. These are the
three integer size attributes supported by nv_getval(). The build
will abort on any (hypothetical?) system where pid_t has a size
other than these three. In that case, the test and nv_getval()
will need to have a new integer size attribute added.
src/cmd/ksh93/data/variables.c:
- Use NV_PID instead of NV_INTEGER for $PPID and ${.sh.pid}. This
fixes the issue.
Other changed files:
- Fix the typecasts to various fmtbase() calls; the first argument
is of type intmax_t (a.k.a. Sflong_t), not long.
- When outputting PIDs with sfprintf(), typecast to Sflong_t and
use %lld to account for possible systems with very large PIDs.
(Though %lld is not in C89/C90, Sfio supports it anyway.)
I made a mistake in sh.reinit() which caused $PPID to be set to the
new process ID, not the parent process ID. This commit fixes it by
introducing, updating and using sh.current_ppid, so we continue to
minimise context switches due to getpid(2)/getppid(2) system calls.
Thanks to Geoff Clare for the report.
The arguments to the binary numerical comparison operators (-eq,
-gt, etc.) in the [[ and test/[ commands are treated as arithmetic
expressions, even if $((...)) is not used. But there is some
seriously incorrect behaviour:
Reproducers (all should output 0/true):
$ [[ 0x0A -eq 10 ]]; echo $?
1
$ [[ 1+0x0A -eq 11 ]]; echo $?
0
$ (set --posix; [[ 010 -eq 8 ]]); echo $?
1
$ (set --posix; [[ +010 -eq 8 ]]); echo $?
0
$ [[ 0xA -eq 10 ]]; echo $?
1
$ xA=10; [[ 0xA -eq 10 ]]; echo $?
0
$ xA=WTF; [[ 0xA -eq 10 ]]; echo $?
ksh: WTF: parameter not set
(POSIX mode enables the recognition of the initial 0 as a prefix
indicating an octal number in arithmetic expressions.)
The cause is the two 'while' loops in this section in test_binop()
in src/cmd/ksh93/bltins/test.c:
502: if(op&TEST_ARITH)
503: {
504: while(*left=='0')
505: left++;
506: while(*right=='0')
507: right++;
508: lnum = sh_arith(left);
509: rnum = sh_arith(right);
510: }
So initial zeros are unconditionally skipped. Ostensibly this is to
disable the recognition of the initial zero as an octal prefix as
well as 0x as a hexadecimal prefix. This would be okay for
enforcing a decimal-only limitation for simple numbers, but to do
this for arithmetic expressions is flagrantly incorrect, to say the
least. (insert standard rant about AT&T quality standards here)
The fix for '[[' is simply to delete the two 'while' loops. But
that creates a problem for the deprecated-but-standard 'test'/'['
command, which also uses the test_binop() function. According to
POSIX, test/[ only parses simple decimal numbers, so octal, etc. is
not a thing. But, after that fix, 'test 08 -eq 10' in POSIX mode
yields true, which is unlike every other shell. (Note that this is
unlike [[ 08 -eq 10 ]], which yields true on bash because '[['
parses operands as arithmetic expressions.)
For test/[ in non-POSIX mode, we don't need to change anything. For
POSIX mode, we should only parse literal decimal numbers for these
operators in test/[, disallowing unexpanded arithmetic expressions.
This makes ksh's POSIX-mode test/[ act like every other shell and
like external .../bin/test commands shipped by OSs.
src/cmd/ksh93/bltins/text.c: test_binop():
- Correct a type mismatch. The left and right hand numbers should
be Sfdouble_t, the maximum double length on the current system,
and the type that sh_arith() returns. Instead, long double
(typeset -lF) values were reduced to double (typeset -F) before
comparing!
- For test/[ in POSIX, only accept simple decimal numbers: use
strtold() instead of sh_arith(). Do skip initial zeros here as
this disables the recognition of the hexadecimal prefix. Throw an
error on an invalid decimal number. Floating point constants are
still parsed, but that's fine as this does not cause any
incompatibility with POSIX.
- For code legibility, move handling of TEST_EQ, etc. to within the
if(op&TEST_ARITH) block. This also allows lnum and rnum to be
local to that block.
We're nearly there!
I intend to release ksh 93u+m/1.0.0 on 2022-08-01, precisely ten
years after the last canonical 93u+ release.
We have a week until then, so here's a release candidate. Please
try as hard as you can to break it, and to help fix known bugs.
As of this commit, the 1.0 branch is feature-frozen and will only
get bugfixes.
src/cmd/ksh93/fun/man:
- Last-minute fix: .man.try_os_man(): do not look for arguments
with / in section 1 and 8; this can cause false positives.
Not only is it C++-specific (and we no longer pretend to be
compatible with C++), it's also z/OS-specific. It seems that
access to z/OS for testing is not available to mere mortals.
The Solaris 10.1 man command happily exits with status 0 on error
and even prints error messages to standard output. This was fixed
in later versions of Solaris. but we should still fix the function
for portability as it should work almost anywhere.
.man.try_os_man() now checks that the standard output of a man
command includes at least three newlines; that's probably a
reliable-enough indicator that it's a man page and not an error
message. Thankfully, all man commands do seem to deactivate the
pager when standard output is not on a terminal (e.g. when catching
standard output with a command substitution).
Somewhat notable changes:
- Remove pointless test commands from Mamfiles.
- Consistent use of NoN macro instead of manual void _STUB_foo(){}
(this is to silence "foo.o has no symbols" warnings from ld).
- Remove src/lib/libast/comp/transition.c; obsolete, does nothing.
- xec.c: Fix off-by-one error in sigreset() used by sh_ntfork():
it tried to (re)set signal 0, which is harmless, but wrong.
(note that sh.st.trapmax is the current max trapped sig + one!)
This autoloadable function activates a feature similar to 'shopt -s
autocd' in bash: type only a directory name to change directory.
It uses the DEBUG trap to work. See the file for details.
This adds a new 'man' function in src/cmd/ksh93/fun/man, meant for
autoload. This integrates the --man self-documentation of ksh
built-in and external AST commands with your system's 'man' command
so you can conveniently use 'man' for all commands, whether
built-in or external. See the file for details.
Since 'command -x' provides xargs-like functionality to repeatedly
run external commands if the argument list is too long for one
invocation, it makes little sense to use with built-ins. So I
decided that 'command -x' should double as a way to guarantee
running an external command. However, it was only bypassing plain
built-ins and not path-bound builtins (the ones that show up with a
virtual directory path name in the output of 'builtin').
src/cmd/ksh93/sh/path.c:
- Before looking for a path-bound built-in, check that the SH_XARG
state bit is not on. This needs to be done in path_absolute() as
well as in path_spawn().
Even tough sh_subtmpfile() should only be relevant to command
substitutions, checking the sh.comsub flag instead of sh.subshell
before calling is not valid in all cases; subshells of command
substitutions run into problems in some oddly specific cases, e.g.,
both 'eval' and an external command must be involved. The ksh
regression tests didn't detect a problem, but both modernish[*] and
shellspec[*] have one regression test failure after that change.
Minimal reproducer, assuming a cat at /bin/cat:
v=$(eval 'print output | /bin/cat')
print -r "v=[$v]"
Actual output:
output
v=[]
Expected output:
v=[output]
This commit makes three interrelated changes.
First, the code for erasing the command line before redrawing it
upon a window size change is simplified and modernised. Instead of
erasing the line with lots of spaces, it now uses the sequence
obtained from 'tput ed' (usually ESC, '[', 'J') to "erase to the
end of screen". This avoids messing up the detection and automatic
redrawing of wrapped lines on terminals such as Apple Terminal.app.
Second, the -b/--notify option is made more usable. When it is on
and either the vi or emacs/gmacs line editor is in use, 'Done' and
similar notifications are now buffered and trigger a command line
redraw as if the window size changed, and the redraw routine prints
that notify buffer between erasing and redrawing the commmnd line.
The effect is that the notification appears to magically insert
itself directly above the line you're typing. (The notification
behaviour when not in the shell line editor, e.g. while running
commands such as external editors, is unchanged.)
Third, a bug is fixed that caused -b/--notify to only report on one
job when more than one terminated at the same time. The rest was
reported on the next command line as if -b were not on. Reproducer:
$ set -b; sleep 1 & sleep 1 & sleep 1 &
This commit also includes a fair number of other window size and
$COLUMNS/$LINES handling tweaks that made all this easier, not all
of which are mentioned below.
src/cmd/ksh93/include/fault.h,
src/cmd/ksh93/sh/fault.c:
- Replace sh_update_columns_lines with a new sh_winsize() function.
It calls astwinsize() and is to be used instead of it, and
instead of nv_getval(LINES) and nv_getval(COLUMNS) calls. It:
- Allows passing one or neither of lines or cols pointers.
- Updates COLUMNS and LINES, but only if they actually changed
from the last values. This makes .set discipline functions
defined for these variables more useful.
- Sets the sh.winch flag, but only if COLUMNS changes. If only
the height changes, the command line does not need redrawing.
src/cmd/ksh93/include/io.h:
- Add sh_editor_active() that allows checking whether one of vi,
emacs or gmacs is active without onerous #if SHOPT_* directives.
src/cmd/ksh93/sh/jobs.c: job_reap():
- Remove the fix backported in fc655f1a, which was really just a
workaround that papered over the real bug.
- Move a check for errno==ECHILD so it is only done when waitpid()
returns an error (pid < 0); the check was not correct because C
library functions that do not error out also do not change errno.
- Move the SH_NOTIFY && SH_TTYWAIT code section to within the
while(1) loop so it is run for each job, not only the
last-processed one. This fixes the bug where only one job was
notified when more than one ended at the same time.
- In that section, check if an editor is active; if so, set the
output file for job_list() to sh.notifybuffer instead of standard
error, list the jobs without the initial newline (JOB_NLFLAG),
and trigger a command line redraw by setting sh.winch.
src/cmd/ksh93/edit/edit.c:
- Obtain not just CURSOR_UP but also ERASE_EOS (renamed from
KILL_LINE) using 'tput'. The latter had the ANSI sequence
hardcoded. Define a couple of TPUT_* macros to make it easier to
deal with terminfo/termcap codes.
- Add get_tput() to make it easier to get several tput values
robustly (with SIGINT blocked, trace disabled, etc.)
- ed_crlf(): Removed. Going by those ancient #ifdefs, nothing that
93u+m will ever run on requires a '\r' before a '\n' to start a
new line on the terminal. Plus, as of 93u+, there were already
several spots in emacs.c and vi.c where it printed a sole '\n'.
- ed_read():
- Simplify/modernise command line erase using ERASE_EOS.
- Between erasing and redrawing, print the contents of the notify
buffer. This has the effect of inserting notifications above
the command line while the user is typing.
src/cmd/ksh93/features/cmds:
- To detect terminfo vs termcap codes, use all three codes we're
currently using. This matters on at least on my system (macOS
10.14.6) in which /usr/bin/tput has incomplete terminfo support
(no 'ed') but complete termcap support!
Reproducer:
$ /bin/sleep 100
^Z[1] + Stopped /bin/sleep 100
$ kill %% <--- no notification shown
$ jobs <--- nothing: it was in fact killed
Expected behaviour:
$ /bin/sleep 100
^Z[1] + Stopped /bin/sleep 100
$ kill %%
[1] + Terminated /bin/sleep 100
In the reproducer, the job in fact gets killed, but no notification
is printed of this fact. This is because notifications now require
the P_BG (background job) flag, and it is not set if a foreground
job is stopped. It should be: stopping moves it to the background.
When the 'sleep' builtin is used instead of the external command,
the notice says 'Done' instead of 'Terminated' because SIGTERM is
being handled via sh_fault() instead of set to SIG_DFL. It remains
to be considered if anything needs to change about that.
src/cmd/ksh93/sh/jobs.c: job_reap():
- Also set a job's P_BG bit when it was stopped.
