1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00
Commit graph

94 commits

Author SHA1 Message Date
Martijn Dekker
0a10e76ccc typeset: add error msgs for incompatible options; improve usage msg
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
2021-01-21 09:36:10 +00:00
Martijn Dekker
6445573d24 typeset: only alloc stack space if needed (re: ff70c27f)
src/cmd/ksh93/bltins/typeset.c:
- Don't call stakalloc(3) unless/until we actually need it to
  construct a new list of arguments.
2021-01-21 08:39:10 +00:00
Lev Kujawski
ff70c27f24
typeset: Fix stack fencepost error, ISO C90 compat (#159)
src/cmd/ksh93/bltins/typeset.c:
- The new_argv[] array was one item too short (should be argc+2).
- Use AST stakalloc(3) to allocate it instead of a dynamic array;
  this restores compatibility with ISO C90.

src/lib/libast/features/standards, src/cmd/INIT/cc.unixware.i386:
- Add support for UnixWare.
- Do not define any standards macros on this system, as on FreeBSD
  and DragonFly BSD.
2021-01-21 07:50:07 +00:00
Martijn Dekker
bb4d6a2ee5 cd: add missing "test.h" include (re: 5ee290c7)
Fixes: https://github.com/ksh93/ksh/issues/158
2021-01-20 22:13:32 +00:00
Martijn Dekker
5ee290c7a8 cd: Fork if $PWD exists but is not actually the PWD (re: d1483150)
Commit d1483150 did not fully fix #153.
Test case from Harald van Dijk that was still failing:

$ mkdir test
$ cd test
$ rmdir $PWD
$ mkdir $PWD
$ ksh -c "(cd /); pwd"
/

Forking a virtual subshell in that case is needed to avoid ending
up in a directory that replaced the PWD, because it will not be
possible for a process to change back to the original directory.

src/cmd/ksh93/bltins/cd_pwd.c:
- When deciding whether to fork, instead of attempting to opendir
  the PWD, compare the inodes $PWD and "." to determine if $PWD
  still actually refers to the current directory. This uses the
  test_inode() function which is also used by 'test foo -ef bar'.

src/cmd/ksh93/tests/subshell.sh:
- Add test based on the above.

Progresses: https://github.com/ksh93/ksh/issues/153
2021-01-20 05:56:38 +00:00
Martijn Dekker
7bab9508aa Fix crash on subshell exit if PWD is inaccessible (re: dd9bc229)
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
2021-01-19 18:47:41 +00:00
Martijn Dekker
4d5e21de80 Fix for compiling with SHOPT_DYNAMIC disabled
src/cmd/ksh93/bltins/typeset.c:
- Correct faulty preprocessor directive logic causing a build
  failure if SHOPT_DYNAMIC is disabled.
2021-01-10 23:02:15 +00:00
Martijn Dekker
4d0b77d398 Revert "Fix SIGALRM core dump (Solaris patch 230-18229654)"
This reverts commit 13e7b262. It caused the regression test for the
'alarm' builtin, introduced in 18b3f4aa, to hang on FreeBSD.
2021-01-09 13:18:00 +00:00
Martijn Dekker
5d7e00a109 cd: validate $OLDPWD (Solaris patch 185-Bug17714341)
This change was pulled in from:
https://raw.githubusercontent.com/oracle/solaris-userland/master/components/ksh93/patches/185-Bug17714341.patch

No public information about the reasons for this change is
available, but it seems reasonable to trust that the Solaris people
found a legitimate need for it.

src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- When determining the old PWD before 'cd', do not trust shp->pwd
  but get and validate the current PWD using path_pwd().
2021-01-08 22:31:16 +00:00
Martijn Dekker
13e7b26202 Fix SIGALRM core dump (Solaris patch 230-18229654)
This should fix the following Solaris bug:
18229654 ksh93 read not reentrant in alarm context dumps core
with the patch taken from:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/230-18229654.patch

Unfortunately the link to the details is inaccessible
as lists.research.att.com is dead.
2021-01-08 18:50:34 +00:00
Martijn Dekker
17ebfbf6a3 Fix I/O redirection in -c script (Solaris patch 280-23332860)
This change is pulled from here:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/280-23332860.patch

Info and reproducers:
https://github.com/att/ast/issues/36

In a -c script (like ksh -c 'commands'), the last command
misredirects standard output if an EXIT or ERR trap is set.
This appears to be a side effect of the optimisation that
runs the last command without forking.

This applies a patch by George Lijo that flags these specific
cases and disables the optimisation.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/bltins/trap.c,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Apply patch as above.

src/cmd/ksh93/tests/io.sh:
- Add the reproducers from the bug report as regression tests.
2021-01-08 15:15:53 +00:00
Martijn Dekker
a3ccff6c75 cd: fix an invalid free() call (Solaris patch 211-21547336)
This change is pulled from here:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/211-21547336.patch

src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- The functions path_pwd() and path_relative() in sh/path.c may
  return a pointer to e_dot[] (".") as a fallback if they fail to
  determine a path. This is a string in read-only memory
  (data/msg.c), so must not be freed. A pointer to that string may
  end up in sh.pwd (== shp->pwd), so b_cd() needs a check for that.
2021-01-08 12:43:19 +00:00
Martijn Dekker
222515bf08 Implement hash tables for virtual subshells (re: 102868f8, 9d428f8f)
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
2021-01-07 22:18:25 +00:00
Martijn Dekker
d1483150ab 'cd': properly ignore $CDPATH if initial component is '.' or '..'
@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
2021-01-05 05:04:24 +00:00
hyenias
88a6baa1a7
Fix floating point numerics having precision of 0 with assignments (#149)
Issuing typeset floating point numerics having a precision of 0
failed as the precision/size was being overwritten with the string
length of the value, e.g. 'typeset -F0 x=5.67' would result in
'typeset -F 4 x=5.6700' as len('5.67') is 4.

src/cmd/ksh93/include/nval.h:

- Created a symbolic name of NV_FLTSIZEZERO to respresent a float
  having a precision/size of 0. NV_FLTSIZEZERO needs to be a
  negative value.

src/cmd/ksh93/bltins/typeset.c:

- In b_typeset(), added code to set tdata.argnum to NV_FLTSIZEZERO
  for E, F, X options.

- In setall(), adjusted code to allow for tp->argnum to be negative.

src/cmd/ksh93/sh/name.c: nv_newattr():

- Adjusted option value only change code to handle NV_FLTSIZEZERO as
  well as changed to directly setting np->nvsize instead of using
  nv_setsize(np,size) as nv_setsize might contain conflicting and/or
  redundant code.

- Added missing conditional check of '!(newatts&NV_INTEGER)' to
  constrain the size==0 code block to justified strings as
  NV_LJUST, NV_RJUST, or NV_ZFILL are only valid for strings if
  NV_INTEGER is not set. This code block was mistakenly setting
  the precision/size value to the length of the value of an
  assignment for floats whereas it should only be performing
  auto assignment length for justified strings.
2020-11-26 13:50:30 +00:00
hyenias
95fe07d869
Improved 'typeset -xu'/'typeset -xl' fix (re: fdb9781e) (#147)
'typeset -xu' and 'typeset -xl' would export the variable but fail
to change case in the value as the check between old and new
attributes did not provide the necesssary insight for lower or
upper case transcoding due to the lower or upper case attribute
being set within typeset.c prior to calling name.c nv_newattr
function.

Previous rhbz#1188377 patch added a conditional check for size==-1
which in effect caused the nv_newattr export code block return
optimization to never be executed as one cannot set any attributes
using the readonly builtin. By altering the size==-1 check to !trans
the export only optimization can run.

Also, the rhbz#1188377 patch altered new_attr function by setting
the new size to oldsize if run by the readonly builtin. The result
of setting size==oldsize allowed the succeeding if statement to
run more frequently and if size was a non-zero value resulted in
nv_setsize resetting the value to what it already was. Investigation
yielded that size was always 0 coming from the readonly builtin.

src/cmd/ksh93/bltins/typeset.c:
- Remove the setting of tdata.argnum to -1 as it is not needed due to
  existing name.c nv_newattr() logic.

src/cmd/ksh93/sh/name.c: nv_newattr():
- Corrected the export only check optimization by using !trans instead
  of using size==-1.
- Removed previous condition check to set size=oldsize if coming from
  the readonly builtin. nv_newattr already had existing logic to
  prevent changing the size via nv_setsize as size is always 0 when
  coming from readonly builtin.
2020-11-26 13:30:24 +00:00
Martijn Dekker
dd9bc22928 Mitigate PWD race condition in non-forking subshells
Virtual/non-forking subshells that change the present working
directory (PWD) with 'cd' suffer from a serious race condition. The
PWD is changed within the same process. This means it may not be
possible to change back to the original PWD when exiting the
subshell, as some other process may destroy the PWD or modify its
permissions in the meantime. ksh did not handle this error
condition at all, so, after exiting a subshell that invoked 'cd',
it could silently end up running the script's following command(s)
in the wrong directory. Which might be 'rm -rf *'. So, ouch.

The proper and obvious fix is never to allow a virtual subshell to
change the PWD, as it can never be guaranteed you can return to a
previous directory. If the PWD is changed in a child process, there
is no need to restore it in the parent process, and this whole
problem is avoided. So subshells really should always fork on
encountering a 'cd' command.

But forking is slow. It is not uncommon for scripts to 'cd' in a
subshell that is run repeatedly in a loop.

There is also the issue of custom builtins that can be added to ksh
via shared libraries. In the standard shell language, 'cd' is the
only command that changes the PWD, so we could just make that
command fork the subshell it is run from. But there's no telling
what a custom builtin might do.

So this commit implements a compromise that will not affect
performance unless there is the pathological condition of a PWD
that has been rendered inaccessible in some way:

1. When entering a virtual subshell, if the parent shell's PWD
proves inaccessible upon saving it, the subshell will now fork into
a separate process, avoiding the unrestorable PWD problem.

2. If some attack renders the parent shell's PWD unrestorable
*after* ksh enters a virtual subshell, ksh will now error out when
exiting it. There is nothing else left to do then. Continuing would
mean running arbitrary commands in the wrong PWD.

src/cmd/ksh93/sh/subshell.c:

- Put all the code/variables only needed for fchdir() behind '#if
  _lib_fchdir'. This makes it clearer what's what.
  (I don't know if there is still any system out there without
  fchdir(3); I haven't found any. The chdir(3) fallback version may
  be removed later as there is no way to make it remotely secure.)

- Fix the attempt to use the O_PATH mode for open(2) as a fallback
  for nonexistent O_SEARCH on Linux. Define _GNU_SOURCE on Linux,
  or <fcntl.h> (which is included indirectly) won't define O_PATH.

- Fix use of O_SEARCH. The code was simply wrong, repeating an
  open(".",O_RDONLY) instead. Since a nonexistent O_SEARCH is now
  redefined as either O_PATH or O_RDONLY, we can simply
  open(".",O_SEARCH) and be done with it.

- Fix fatal error handling. Introduce fatal error condition for
  failure to fchdir(3) back to the parent's PWD; rename 'duped' to
  'fatalerror' and use it for error numbers; save and restore errno
  on fatal error so the message will report the cause. (We must
  call errormsg() near the end of sh_subshell() to avoid crashes.)

- If open(".",O_SEARCH) was not able get a file descriptor to our
  PWD on entry, then call sh_subfork() immediately before running
  the subshell commands. (Forking earlier causes a crash.)

- When restoring the PWD, if fchdir(3) fails, do *not* fall back to
  chdir(3). We already know the PWD is inaccessible, so if chdir(3)
  "succeeds" then, it's very likely to be a substitute injected by
  an attacker.

src/cmd/ksh93/bltins/cd_pwd.c:

- If we don't have fchdir(3), then sh_subshell() must fall back to
  chdir(2) to restore the PWD. That is highly vulnerable, as a
  well-timed rename would allow an attacker to usurp the PWD. We
  can't do anything about that if some custom builtin changes the
  PWD, but we can at least make 'cd' always fork a subshell, which
  slows down ksh but removes the need for the parent shell ever to
  restore the PWD. (There is certainly no popular system where this
  is relevant and there might not be any such current system.)

This commit adds no regression test because a portable regression
test is not really doable. Different kernels, external /bin/pwd
utilities, etc. all have quite different behaviour under the
pathological condition of an inaccessible PWD, so both the
before-fix and the after-fix behaviour differs. See link below.

Resolves: https://github.com/ksh93/ksh/issues/141
Thanks to Stéphane Chazelas for the bug report.
2020-10-07 00:52:11 +02:00
hyenias
6697edba1c
Enforce integer base limits of 2 to 64 (#139)
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- For integer bases change argnum check to default values that
  are < 2 or > 64 to 10 instead of allowing invalid base values
  that ksh cannot process.

src/cmd/ksh93/bltins/typeset.c: setall():
- Remove argnum check for integer base of 1 as base cannot be 1.
- Relocate strlen(name) to inside of conditional check for
  np->nvfun as this code does not need to run all.
- Remove no-op oldname code

src/cmd/ksh93/tests/attributes.sh:
- Add typeset -i base bounds checks to default base 10
2020-10-04 10:18:34 +01:00
Martijn Dekker
48ba6964ad Turn off SH_INTERACTIVE state flag in subshells
By definition, subshells are never interactive, so they should
disable behaviour associated with interactive shells even if the
main shell is interactive.

Most visibly, running a background job from a subshell like
	( some_command & )
now no longer prints a job ID that you cannot use in the main shell.
This behaviour change matches pdksh/mksh, bash, zsh, dash, et al.

Prior discussion:
https://www.mail-archive.com/austin-group-l@opengroup.org/msg06456.html
(plus the preceding thread)

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Before running the command(s) in the subshell using sh_exec(),
  turn off the SH_INTERACTIVE shell state flag. (No need to add
  code to restore it as this function already saves and restores
  the entire shell state.)

src/cmd/ksh93/bltins/misc.c: b_bg():
- If there is no job control when using 'bg', 'fg' or 'disown',
  always print the "no job control" error message and not only if
  the shell is in the interactive state. This is also what
  pdksh/mksh, bash and zsh do.
2020-10-02 08:07:28 +02:00
Martijn Dekker
fdb9781ebb Fix 'typeset -xu', 'typeset -xl' (rhbz#1188377)
'typeset -xu' and 'typeset -xl' would export the variable but fail
to change case in the value under certain conditions.

Original patch:
642af4d6/f/ksh-20120801-xufix.patch

This applies the patch essentially without change and adds a
regression test based on the reproducer provided in the RH bug.

Unfortunately there is no description of how the patch works and
it's a little obscure to me. As far as I can figure out, the cause
of the problem was that nv_newattr() erroneously processed a
nonexistent size option-argument such as what can be given to
options like typeset -F, e.g. typeset -F3 for 3 digits after the
dot. A nonexistent size argument is represented by the value of -1.
2020-09-30 03:06:54 +02:00
Martijn Dekker
18b3f4aa28 combining alarm and IFS caused segfault (rhbz#1176670)
The undocumented alarm builtin executes actions unsafely so that
'read' with an IFS assignment crashed when an alarm was triggered.

This applies an edited version of a Red Hat patch:
642af4d6/f/ksh-20120801-alarmifs.patch

Prior discussion:
https://bugzilla.redhat.com/1176670

src/cmd/ksh93/bltins/alarm.c:
- Add a TODO note based on dgk's 2014 email cited in the RH bug.
- When executing the trap function, save and restore the IFS table.

src/cmd/ksh93/sh/init.c: get_ifs():
- Remove now-unnecessary SHOPT_MULTIBYTE preprocessor directives as
  8477d2ce lets the compiler optimise out multibyte code if needed.
- Initialise the 0 position of the IFS table to S_EOF. This
  corresponds with the static state tables in data/lexstates.c.

src/cmd/ksh93/tests/builtins.sh:
- Crash test.
2020-09-27 03:03:48 +02:00
Johnothan King
8a34fc40e6
whence -f: ignore functions (#137)
According to 'whence --man', 'whence -f' should ignore functions:
  -f              Do not check for functions.

Right now this is only accomplished partially. As of commit
a329c22d 'whence -f' avoids any output when encountering a
function (in ksh93u+ 'whence -f' has incorrect output). The
return value is still wrong though:

$ foo() { true; }
$ whence -f foo; echo $?
0

This commit fixes the return value and makes 'type -f' error out
when given a function (like in Bash).

src/cmd/ksh93/bltins/whence.c:
- If -f was passed, set 'cp' to NULL since functions should be
  ignored (as documented).
- Simplify return value by avoiding bitwise logic.

src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for 'whence -f' and 'type -f'.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2020-09-26 19:26:18 +01:00
Martijn Dekker
3050bf28bc whence -v/-a: report path to autoloadable functions
Since at least 1999, whence -v on pdksh (and its successor mksh)
reports the path where an autoloadable function may be found:

  $ mkdir ~/fun; FPATH=~/fun
  $ echo 'myfn() { echo hi; }' >~/fun/myfn
  $ whence -v myfn
  myfn is a undefined (autoload from /home/user/fun/myfn) function

Whereas ksh93 only reports, rather uselessly:

  myfn is an undefined function

As of this commit, whence -v/-a on ksh 93u+m does the same as
pdksh, but with correct grammar:

  myfn is an undefined function (autoload from /home/user/fun/myfn)

This may be a small violation of my own "no new features" policy
for 93u+m, but I couldn't resist. This omission has been annoying
me, and it's just embarrassing to lack a pdksh feature :)

src/cmd/ksh93/include/path.h,
src/cmd/ksh93/data/msg.c:
- Add e_autoloadfrom[] = " (autoload from %s)" message.

src/cmd/ksh93/bltins/whence.c: whence():
- Report the path (if any) when reporting an undefined function.
  This needs to be done in two places:
  1. When a function has been explicitly marked undefined with
     'autoload', we need to do a quick path_search() loop to find
     the path. (These undefined functions take precedence over
     regular commands, so are reported first.)
  2. When a function is not explicitly autoloaded but merely
     available in $FPATH, that path search was already done, so all
     we need to do is report it. (These are reported last.)
  Note that the output remains as on 93u+ if no function definition
  file is found on $FPATH. This is also like pdksh/mksh.

src/cmd/ksh93/data/builtins.c:
- Bump 'whence' version date. The inline docs never detailed very
  exactly what 'whence -v' reports, so no need for further edits.

src/cmd/ksh93/tests/path.sh:
- Regress-test the new whence behaviour plus actual autoloading,
  including the command override behaviour of autoloaded functions.
2020-09-25 17:45:40 +02:00
Martijn Dekker
a329c22dba Multiple 'whence' and path search fixes
Hopefully this doesn't introduce new bugs, but it does fix at
least the following:

1. When whence -v/-a found an "undefined" (i.e. autoloadable)
   function in $FPATH, it actually loaded the function as a side
   effect of reporting on its existence (!). Now it only reports.

2. 'whence' will now canonicalise paths properly. Examples:
	$ whence ///usr/lib/../bin//./env
	/usr/bin/env
	$ (cd /; whence -v dev/../usr/bin//./env)
	dev/../usr/bin//./env is /usr/bin/env

3. 'whence' no longer prefixes a spurious double slash when doing
   something like 'cd / && whence bin/echo'. On Cygwin, an initial
   double slash denotes a network server, so this was not just a
   cosmetic problem.

4. 'whence -a' now reports a "tracked alias" (a.k.a. hash table
   entry, i.e. cached $PATH search) even if an actual alias by the
   same name exists. This needed fixing because in fact the hash
   table entry continues to be used when bypassing the alias.
   Aliases and "tracked aliases" are not remotely the same thing;
   confusing nomenclature is not a reason to report wrong results.

5. When using 'hash' or 'alias -t' on a command that is also a
   builtin to force caching a $PATH search for the external
   command, 'whence -a' double-reported the path:
	$ hash printf; whence -a printf
	printf is a shell builtin
	printf is /usr/bin/printf
	printf is a tracked alias for /usr/bin/printf
   This is now fixed so that the second output line is gone.
   Plus, if there were multiple versions of the command on $PATH,
   the tracked alias was reported at the end, which is the wrong
   order. This is also fixed.

src/cmd/ksh93/bltins/whence.c: whence():
- Refactor the do...while loop that handles whence -v/-a for path
  searches in such a way that the code actually makes sense and
  stops looking like higher esotericism. Just doing this fixed #2,
  #4 and #5 above (the latter two before I even noticed them). For
  instance, the path_fullname() call to canonicalise paths was
  already there; it was just never used.
- Remove broken 'notrack' flaggery for deciding whether to report a
  hash table entry a.k.a. "tracked alias"; instead, check the hash
  table (shp->track_tree).

src/cmd/ksh93/sh/path.c:
- path_search(): Re #3: When prefixing the PWD, first check if
  we're in '/' and if so, don't prefix it; otherwise, adding the
  next slash causes an initial double slash. (Since '/' is the only
  valid single-character absolute path, all we need to do is check
  if the second character pwd[1] is non-null.)
- path_search(): Re #1: Stop autoloading when called by 'whence':
  * The 'flag==2' check to avoid autoloading a function was
    broken. The flag value is 2 on the first whence() loop
    iteration, but 3 on subsequent ones. Change to 'flag >= 2'.
  * However, this only fixes it if the function file does not have
    the x permission bit, as executable files are handled by
    path_absolute() which unconditionally autoloads functions!
    So, pass on our flag parameter when callling path_absolute().
- path_absolute(): Re #1: Add flag parameter. Do not autoload
  functions if flag >= 2.

src/cmd/ksh93/include/path.h,
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Re #1: Update path_absolute() calls, adding a 0 flag parameter.

src/cmd/ksh93/include/name.h:
- Remove now-unused pathcomp member from union Value. It was
  introduced in 99065353 to allow examining the value of a tracked
  alias. This commit uses nv_getval() instead.

src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/path.sh:
- Add and tweak various related tests.

Fixes: https://github.com/ksh93/ksh/issues/84
2020-09-20 07:56:09 +02:00
Johnothan King
7e7f137245
Fix a crash on unsetting preset alias (re: ddaa145b) (#133)
The following set of commands caused ksh to crash:

$ unalias history; unalias r
Memory fault

When ksh is compiled with -D_std_malloc, the crash always
occurs when the 'r' alias is removed with 'unalias r',
although with vmalloc 'unalias history' must be run first
for the crash to occur. With the native malloc, the crash
message is also different:

$ unalias history; unalias r
free(): invalid pointer
Abort

This crash happens because when an alias is unset, _nv_unset
removes the NV_NOFREE flag which results in an invalid use
of free(3) as nv_isattr no longer detects NV_NOFREE afterward.
The history and r aliases shouldn't be freed from memory by
nv_delete because those aliases are given the NV_NOFREE attribute.

src/cmd/ksh93/bltins/typeset.c:
- Save the state of NV_NOFREE for aliases to fix the crash
  caused by 'unalias r'.

src/cmd/ksh93/tests/alias.sh:
- Use unalias on both history and r to check for the crash.
  'unalias -a' can't be used to replicate the crash.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2020-09-18 11:17:20 +01:00
Martijn Dekker
ddaa145b3d Reinstate 'r' and 'history' as preset aliases for interactive ksh
Following a community discussion, it became clear that 'r' is
particularly problematic as a regular builtin, as the name can and
does conflict with at least one legit external command by that
name. There was a consensus against removing it altogether and
letting users set the alias in their login scripts. However,
aliases are easier to bypass, remove or rename than builtins are.
My compromise is to reinstate 'r' as a preset alias on interactive
shells only, along with 'history', as was done in 17f81ebe before
they were converted to builtins in 03224ae3. So this reintroduces
the notion of predefined aliases to ksh 93u+m, but only for
interactive shells that are not initialised in POSIX mode.

src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
- Restore aliases.c containing shtab_aliases[], a table specifying
  the preset aliases.

src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/sh/init.c:
- Rename inittree() to sh_inittree() and make it extern, because we
  need to use it in main.c (sh_main()).

src/cmd/ksh93/sh/main.c: sh_main():
- Init preset aliases from shtab_aliases[] only if the shell is
  interactive and not in POSIX mode.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/alias.sh:
- unall(): When unsetting an alias, pass on the NV_NOFREE attribute
  to nv_delete() to avoid an erroneous attempt to free a preset
  alias from read-only memory. See: 5d50f825

src/cmd/ksh93/data/builtins.c:
- Remove "history" and "r" entries from shtab_builtins[].
- Revert changes to inline fc/hist docs in sh_opthist[].

src/cmd/ksh93/bltins/hist.c: b_hist():
- Remove handling for 'history' and 'r' as builtins.

src/cmd/ksh93/sh.1:
- Update accordingly.

Resolves: https://github.com/ksh93/ksh/issues/125
2020-09-11 21:35:45 +02:00
Martijn Dekker
092b90da81 Fix BUG_LOOPRET2 and related return/exit misbehaviour
The 'exit' and 'return' commands without an argument failed to pass
down the exit status of the last-run command when incorporated in a
block with redirection, &&/|| list, 'case' statement, or 'while',
'until' or 'for' loop.

src/cmd/ksh93/bltins/cflow.c:
- Use $?, which is sh.savexit a.k.a. shp->savexit, as the default
  exit status value if there is no argument, instead of
  shp->oldexit. This fixes the default exit status behaviour to
  match POSIX and other shells.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- Remove now-unused sh.oldexit (a.k.a. shp->oldexit) private struct
  member. It appeared to fulfill the same function as sh.savexit,
  but in a slightly broken way.
- Move the savexit/$? declaration from the _SH_PRIVATE part of the
  struct definition to the public API part. Since $? uses this,
  it's clearly a publicly exposed value already, and this is
  generally the one to use. (If anything, it's exitval that should
  have been private.) This declares savexit right next to exitval,
  rewriting the comments to clarify the difference between them.

src/cmd/ksh93/sh/fault.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Remove assignments to shp->oldexit.

src/cmd/ksh93/tests/basic.sh:
- Add thorough regression tests for the default exit status
  behaviour of 'return' and 'exit' in various lexical contexts.
- Verify that 'for' and 'case' without any command, as well as a
  lone redirection, still correctly reset the exit status to 0.

Fixes: #117
2020-09-09 20:02:20 +02:00
Martijn Dekker
e1c41bb2de Fix subshell leak for 3 special variables (re: 417883df, bd3e2a80)
Using a process of elimination I've identified ${.sh.level}
(SH_LEVELNOD) as the cause of the crash. This node apparently
cannot be copied or moved without destabilising the shell. It
contains the current depth of function calls and it cannot be
changed by assignment, so this is not actually a problem.
Meanwhile, this commit re-fixes it for the other three.

src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for L_ARGNOD,
  SH_SUBSCRNOD and SH_NAMENOD. 'add' now has 3 modes (0, 1, 2).
- The test for a ${ subshare; } was actually wrong. sp->subshare is
  a saved backup value. We must test shp->subshare. (re: a9de50bf)

src/cmd/ksh93/bltins/typeset.c:
- setall(): Update the mode 3 sh_assignok() call.

src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables except
  ${.sh.level}.
2020-09-05 20:47:03 +02:00
Martijn Dekker
417883dfdd Revert "Fix subshell leak for 4 special variables (re: bd3e2a80)"
This reverts commit b3d37b00b0.
While ksh's own regression test suite passed just fine, when
running the modernish[*] regression tests uite, ksh either froze
hard (needing SIGKILL) or threw a spurious syntax error.
Cause unknown, but I'm certainly reverting until I find out.

This reintroduces a subshell leak for four special variables.

[*] https://github.com/modernish/modernish
2020-09-05 16:48:17 +02:00
Martijn Dekker
b3d37b00b0 Fix subshell leak for 4 special variables (re: bd3e2a80)
The following special variables leaked out of a subshell:
$_, ${.sh.name}, ${.sh.level}, ${.sh.subscript}.
This was due to a faulty optimisation in sh_assignok().
bd3e2a80 fixed that in part, this fixes the rest.

src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for these four
  special variables. The 'add' param reverts to a simple boolean.
- The test for a ${ subshare; } was actually wrong. sp->subshare is
  a saved backup value. We must test shp->subshare. (re: a9de50bf)

src/cmd/ksh93/bltins/typeset.c:
- setall(), unall(): Update sh_assignok() calls.

src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables.

Closes: #122
2020-09-05 14:38:44 +02:00
Martijn Dekker
f9c127e39e Remove legacy code for older libast versions
Since ksh 93u+m comes bundled with libast 20111111, there's no need
to support older versions, so this is another cleanup opportunity.

src/cmd/ksh93/include/defs.h:
- Throw an #error if AST_VERSION is undefined or < 20111111.
  (Note that _AST_VERSION is the same as AST_VERSION, but the
  latter is newer and preferred; see src/lib/libast/features/api)

All other changed files:
- Remove legacy code for versions older than the currently used
  versions, which are:
  _AST_VERSION    20111111
  ERROR_VERSION   20100309
  GLOB_VERSION    20060717
  OPT_VERSION     20070319
  SFIO_VERSION    20090915
  VMALLOC_VERSION 20110808
2020-09-04 02:31:39 +02:00
Martijn Dekker
8d7f616e75 Remove abandoned SHOPT_ENV experiment
SHOPT_ENV is an undocumented compile-time option implementing an
experimental method for handling environment variables, which is
implemented in env.h and env.c. There is no mention in the docs or
Makefile, and no mention in the mailing list archives. It adds no
new functionality, but at first glance it's a clean-looking
interface.

However, unfortunately, it's broken. Compiling with -DSHOPT_ENV
added to CCFLAGS causes bin/shtests to show these regressions:

functions.sh[341]: export not restored name=value function call -- expected 'base', got ''
functions.sh[1274]: Environment variable is not passed to a function
substring.sh[236]: export not restored name=value function call
variables.sh[782]: SHLVL should be 3 not 2

In addition, 'export' stops working on unset variables.

In the 93v- beta this code is still present, unchanged, though 93v-
made lots of incompatible changes. By the time ksh2020 noticed it,
it was no longer compiling, so it probably wasn't compiling in the
93v- beta either. Discussion: https://github.com/att/ast/issues/504
So the experiment was already abandoned by D. Korn and his team.

Meanwhile it was leaving sh/name.c with two versions of several
enviornment-related functions, and it's not clear which one is
actually compiled without doing detective work tracing header files
(most of the code was made conditional on _ENV_H, which is defined
in env.h, which is included by defs.h if SHOPT_ENV is defined).
This actively hinders understanding of the codebase. And any
changes to these functions would need to be implemented twice.

src/cmd/ksh93/include/env.h,
src/cmd/ksh93/sh/env.c:
- Removed.

src/cmd/ksh93/DESIGN,
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile:
- Update accordingly.

All other changed files:
- Remove deactivated code behind SHOPT_ENV and _ENV_H.
2020-09-02 16:09:57 +01:00
Martijn Dekker
55f0f8ce52 -o posix: disable '[ -t ]' == '[ -t 1 ]' hack
On ksh93, 'test -t' is equivalent to 'test -t 1' (and of course
"[ -t ]" is equivalent to "[ -t 1 ]").

This is purely for compatibility with ancient Bourne shell
breakage. No other shell supports this. ksh93 should probably keep
it for backwards compatibility, but it should definitely be
disabled in POSIX mode as it is a violation of the standard; 'test
-t' is an instance of 'test "$string"', which tests if the string
is empty, so it should test if the string '-t' is empty (quod non).

This also replaces the fix for 'test -t 1' in a command
substitution with a better one that avoids forking (re: cafe33f0).

src/cmd/ksh93/sh/parse.c:
- qscan(): If the posix option is active, disable the parser-based
  hack that converts a simple "[ -t ]" to "[ -t 1 ]".

src/cmd/ksh93/bltins/test.c:
- e3(): If the posix option is active, disable the part of the
  compatibility hack that was used for compound expressions
  that end in '-t', e.g. "[ -t 2 -o -t ]".
- test_unop(): Remove the forking fix for "[ -t 1 ]".

src/cmd/ksh93/edit/edit.c:
- tty_check(): This function is used by "[ -t 1 ]" and in other
  contexts as well, so a fix here is more comprehensive. Forking
  here would cause a segfault, but we don't actually need to. This
  adds a fix that simply returns false if we're in a virtual
  subshell that is also a command substitution. Since command
  substitutions always fork upon redirecting standard output within
  them (making them no longer virtual), it is safe to do this.

src/cmd/ksh93/tests/bracket.sh
- Add comprehensive regression tests for test/[/[[ -t variants in
  command substitutions, in simple and compound expressions, with
  and without redirecting stdout to /dev/tty within the comsub.
- Add tests verifying that -o posix disables the old hack.
- Tweak other tests, including one that globally disabled xtrace.
2020-09-01 20:24:44 +01:00
Martijn Dekker
77beec1e0d restore 'test --man --' oddness (re: fa6a180f)
Following a community objection to its removal, the inline 'test'
manual page along with its strange method of invocation is
restored. I've taken the opportunity to correct several mistakes,
add some missing info, do some copy-editing, and document the way
to get these docs in the main (k)sh.1 manual.

Discussion:
fa6a180f (commitcomment-41897553)
2020-08-31 23:43:22 +01:00
Martijn Dekker
fa6a180fdd test/[: remove effectively inaccessible self-doc
Did you know that you could get a manual page for the 'test'/'['
builtin command using one of these strange command lines?

	test --man --

	[ --man -- ]

Neither did I. It's not documented or mentioned anywhere (and this
syntax violates POSIX). So nobody knows about it, which makes that
documentation useless. (The regular --man option doesn't work
because that would break 'test'.) I only found out how to invoke it
when I understood what the uncommented C code handling this does.

The test/[ command's self-documentation is unmaintained since 2003
and somewhat incomplete. It's also mostly redundant with the
documentation on Conditional Expressions in the main (k)sh.1 manual
page. But unlike the latter, this is resident in RAM, wasting
working memory in every shell process.

src/cmd/ksh93/sh.1:
- Add documentation for 'test'/'[' commands (yes, they were not
  mentioned in the main manual page until now), describing them
  in terms of differences from '[[' and recommending the latter.

src/cmd/ksh93/include/test.h,
src/cmd/ksh93/bltins/test.c,
src/cmd/ksh93/data/testops.c:
- Remove RAM-resident --man doc for test/[ command.
- Remove the bizarre option parsing that allowed invoking it.
2020-08-30 08:08:01 +01:00
Martijn Dekker
cd2cf236c2 test/[: use a shell state bit (re: 7003aba4)
Instead of a global 'sh_in_test_builtin' integer flag, it is nicer
to use the mechanism for shell state bits, which was designed for
this sort of thing.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/defs.c:
- Remove global sh_in_test_builtin integer.
- Define new SH_INTESTCMD state bit.

src/cmd/ksh93/bltins/test.c: _ERROR_exit_b_test(), b_test():
- Use the new state bit.
2020-08-30 05:33:59 +01:00
Martijn Dekker
42301639d6 '#if 0' cleanup
This removes various blocks of uncommented experimental code that
was disabled using '#if 0' or '#if 1 ... #else' directives. It's
hard or impossible to figure out what the thoughts behind them
might have been, and we can really do without those distractions.
2020-08-30 04:51:20 +01:00
Martijn Dekker
f8feed1bd2 SHOPT_MULTIBYTE-related cleanup (re: 8477d2ce)
As of 8477d2ce, the mbwide() macro (which tests if we're in a
multibyte locale, i.e. UTF-8) is redefined as a constant 0 if we're
compiling without SHOPT_MULTIBYTE. See src/cmd/ksh93/include/defs.h

The other multibyte macros use mbwide() as well, so they all revert
to the single-byte fallbacks in that case, and the multibyte code
in them is never compiled. See src/lib/libast/include/ast.h

Consequently we can now do a bit of cleanup and get rid of many of
the '#if SHOPT_MULTIBYTE' directives, as the compiler optimiser
will happily remove the multibyte-specific code. This increases the
legibility of the ksh code.

I'm taking the opportunity to fix a few typos and whitespace
formatting glitches as well.
2020-08-30 04:50:57 +01:00
Martijn Dekker
56805b25af Fix leak and crash upon defining functions in subshells
A memory leak occurred upon leaving a virtual subshell if a
function was defined within it. If this was done more than 32766
(= 2^15-2 = the 'short' max value - 1) times, the shell crashed.
Discussion and reproducer: https://github.com/ksh93/ksh/issues/114

src/cmd/ksh93/sh/subshell.c: table_unset():
- A subshell-defined function was never freed because a broken
  check for autoloaded functions (which must not be freed[*]). It
  looked for an initial '/' in the canonical path of the script
  file that defined the function, but that path is also stored for
  regular functions. Now use a check that executes nv_search() in
  fpathdict, the same method used in _nv_unset() in name.c for a
  regular function unset.

src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- Fix an additional memory leak introduced in bd88cc7f, that caused
  POSIX functions (which are run with b_dot_cmd() like dot scripts)
  to leak extra. This fix avoids both the crash fixed there and the
  memory leak by introducing a 'tofree' variable remembering the
  filename to free. Thanks to Johnothan King for the patch.

src/lib/libast/include/stk.h,
src/lib/libast/misc/stk.c,
src/lib/libast/man/stk.3,
src/lib/libast/man/stak.3:
- Make the stack more resilient by extending the stack reference
  counter 'stkref' from (signed) short to unsigned int. On modern
  systems with 32-bit ints, this extends the maximum number of
  elements on a stack from 2^15-1==32767 to 2^32-1==4294967295.
  The ref counter can never be negative, so there is no reason for
  signedness. sizeof(int) is defined as the size of a single CPU
  word, so this should not affect performance at all.
     On a 16-bit system (not that ksh still compiles there), this
  doubles the max number of entries to 2^16-1=65535.

src/cmd/ksh93/tests/leaks.sh:
- Add leak regression tests for ksh functions, POSIX functions, dot
  scripts run with '.', and dot scripts run with 'source'.

src/cmd/ksh93/tests/path.sh:
- Add an output builtin with a redirect to an autoloaded function
  so that a crash[*] is triggered if the check for an autoloaded
  function is ever removed from table_unset(), as was done in ksh
  93v- (which crashed).

[*] Freeing autoloaded functions after leaving a virtual subshell
    causes a crashing bug: https://github.com/att/ast/issues/803

Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Fixes: https://github.com/ksh93/ksh/issues/114
2020-08-14 00:25:31 +01:00
Martijn Dekker
cadd1a81dc printf %#H: tweak writing unreserved chars (re: 8477d2ce)
src/cmd/ksh93/bltins/print.c:
- If in UTF-8 locale, only bother to check for unreserved char if
  the character is ASCII (< 128), and write unreserved chars with
  a simple stakputc().
2020-08-13 04:51:52 +01:00
Martijn Dekker
e01801572d printf %H: fix/reduce encoding into entities (re: 8477d2ce)
The &nbsp; entity is not valid in XML, only in HTML. Since we must
be compatible with both, it can't be used. Thanks to Andras Farkas
for the bug report.

In addition, the generation of numeric entities for unprintable
characters was only valid while processing UTF-8 text while in a
UTF-8 locale. In all other conditions it produced invalid results.
This is not worth trying to fix.

Discussion:
https://groups.google.com/d/msgid/korn-shell/CAA0nTRta%3DPbOYduyBv%3DXCzumTcUCU8Lki%3DQQf2O8Erk2BFvO1g%40mail.gmail.com

src/cmd/ksh93/bltins/print.c:
- Remove conversion to &nbsp; entity.
- Remove conversion of non-graph characters to numeric entities.
  Convert only the 5 semantically meaningful characters: < > & " '

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/string.c:
- We don't need sh_isprint() in print.c anymore, so turn it back
  into a static function.

src/cmd/ksh93/tests/builtins.sh:
- Update and trim regression tests.
2020-08-11 08:16:27 +01:00
Martijn Dekker
8477d2ce22 printf: Fix HTML and URI encoding (%H, %#H)
This applies a number of fixes to the printf formatting directives
%H and %#H (as well as their equivalents %(html)q and %(url)q):
1. Both formatters have been made multibyte/UTF-8 aware, and no
   longer delete multibyte characters. Invalid UTF-8 byte sequences
   are rendered as ASCII question marks.
2. %H no longer wrongly encodes spaces as non-breaking spaces
   (&nbsp;) and instead correctly encodes the UTF-8 non-breaking
   space as such.
3. %H now converts the single quote (') to '%#39;' instead of
   '&apos;' which is not a valid entity in all HTML versions.
4. %#H failed to encode some reserved characters (e.g. '?') while
   encoding some unreserved ones (e.g. '~'). It now percent-encodes
   all characters except those 'unreserved' as per RFC3986 (ASCII
   alphanumeric plus -._~).

Prior discussion:
ce8d1467-4a6d-883b-45ad-fc3c7b90e681%40inlv.org

src/cmd/ksh93/include/defs.h:
src/cmd/ksh93/sh/string.c:
- defs.h: If compiling without SHOPT_MULTIBYTE, redefine the
  mbwide() macro (which tests if we're in a multibyte locale) as 0.
  This lets the compiler optimiser do the work that would otherwise
  require a lot of tedious '#if SHOPT_MULTIBYTE' directives.
- string.c: Remove some now-unneeded '#if SHOPT_MULTIBYTE' stuff.
- defs.h, string.c: Rename is_invisible() to sh_isprint(), invert
  the boolean return value, and make it an extern for use in
  fmthtml() -- see below. If compiling without SHOPT_MULTIBYTE,
  simply #define sh_isprint() as equivalent to isprint(3).
- defs.h: Add URI_RFC3986_UNRESERVED macro for fmthtml() containing
  the characters "unreserved" for purposes of URI percent-encoding.

src/cmd/ksh93/bltins/print.c: fmthtml():
- Remove kludge that skipped all multibyte characters (!).
- Complete rewrite to implement fixes described above.
- Don't bother with '#if SHOPT_MULTIBYTE' directives (see above).

src/cmd/ksh93/data/builtins.c:
- sh_optprintf[]: %H: Add single quote to encoded chars doc.
- Edit credits and bump version date.

src/cmd/ksh93/tests/builtins.sh:
- Update and tweak old regression tests.
- Add a number of new tests for UTF-8 HTML and URI encoding, which
  are only run when running tests in a UTF-8 locale (shtests -u).
2020-08-10 22:51:55 +01:00
Martijn Dekker
be5ea8bbb2 redirect: check args before executing redirections (re: 7b82c338)
The 'redirect' builtin command did not error out before executing
any valid redirections. For example, 'redirect ls >foo.txt' issued
an "incorrect syntax" error, but still created 'foo.txt' and left
standard output permanently redirected to it.

src/cmd/ksh93/sh/xec.c: sh_exec():
- If we have redirections (io != NULL), and the command is
  SYSREDIR, then check for arguments and error out if there are
  any, before calling sh_redirect() to execute redirections.
  (Note, the other check for arguments in b_exec() in bltins/misc.c
  must be kept, as that applies if there are no redirections.)

src/cmd/ksh93/sh/io.c: sh_redirect():
- Edit comments to better explain what the flag values do.

src/cmd/ksh93/bltins/misc.c:
- Add a dummy b_redirect() function declaration "for the dictionary
  generator" as has historically been done for other builtins that
  share one C function. I'm not sure what that dictionary generator
  is supposed to be, but this also improves greppability.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Fix misleading "I/O redirection arguments" term. I/O redirections
  are not arguments at all; no argument parser ever sees them.

src/cmd/ksh93/tests/io.sh:
- Test both conditions that should make 'redirect' produce an
  "incorrect syntax" error.
- Test that any redirections are not executed if erroneous
  non-redirection arguments exist.

src/cmd/ksh93/tests/builtins.sh:
- "... should show usage info on unrecognized options" test:
  Because 'redirect' now refuses to process redirections on error,
  the error message was not captured. The fix is to run the builtin
  in a braces block and add the redirection to the block.
2020-08-09 00:47:22 +01:00
Johnothan King
49ae483574
Make liblist an extern to fix dtksh compile (#108)
The liblist variable needs to be an extern for dtksh to build.
Quote from CDE developer Chase:
we use an old function that no longer appears in kornshell,
sh_getliblist, it seems to be replaced by the function sh_getlib,
which is fine, but it seems to return a "Shbltin_f" type, which I
can't seem to find any information on what it is. We need the void
pointer dlsym provides for some widget init stuff, I tried making
liblist an extern, but it kept giving me an error about libcomp_t
being undefined.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/include/shell.h:
- Fix the compiler error reported above by moving the type definition
  for Libcomp_t to shell.h.
- Make liblist an extern since findsym.c in dtksh needs it to build.
  The old sh_getliblist function doesn't need to be reintroduced
  since the only purpose it served was to workaround the problem
  of liblist being a static variable. Now that liblist is an extern,
  dtksh fsym can use liblist directly to avoid sh_getliblist.

dtksh findsym.c:
https://sourceforge.net/p/cdesktopenv/code/ci/2.3.2/tree/cde/programs/dtksh/findsym.c
2020-08-05 22:18:22 +01:00
Martijn Dekker
a2f13c19f2 Fix typeset attributes -a, -A, -l, -u leaking out of subshells
If an array or upper/lowercase variable was declared with a null
initial value within a virtual/non-forked subshell, like:
	( typeset -a foo; ... )
	( typeset -A foo; ... )
	( typeset -l foo; ... )
	( typeset -u foo; ... )
then the type declaration leaked out of the subshell into the
parent shell environment, though without any values that may
subsequently have been assigned.

src/cmd/ksh93/bltins/typeset.c: setall():
- When deciding whether to create a virtual subshell scope for a
  variable, use sh_assignok(), which was actually designed for the
  purpose, instead of _nv_unset(). This allows getting rid of a
  tangled mess of special-casing that never worked quite right.

src/cmd/ksh93/tests/arrays.sh:
- Add regression tests checking that array declarations don't leak
  out of virtual subshells.

src/cmd/ksh93/tests/attributes.sh:
- Add regression tests for combining the 'export' and 'readonly'
  attributes with every other possible typeset attribute on unset
  variables. This also includes a subshell leak test for each one.

Fixes: https://github.com/ksh93/ksh/issues/88
2020-07-26 02:41:12 +01:00
Johnothan King
1bc2c74c74
Fix how unrecognized options are handled in 'sleep' and 'suspend' (#93)
When a builtin is given an unrecognized option, the usage information
for that builtin should be shown as 'Usage: builtin-name options'. The
sleep and suspend builtins were an exception to this. 'suspend' would
not show usage information and sleep wouldn't exit on error:

$ suspend -e
/usr/bin/ksh: suspend: -e: unknown option
$ time sleep -e 1
sleep: -e: unknown option

real	0m1.00s
user	0m0.00s
sys	0m0.00s

src/cmd/ksh93/bltins/sleep.c:
- Show usage information and exit when sleep is given an unknown
  option. This bugfix was backported from ksh2020: https://github.com/att/ast/pull/1024

src/cmd/ksh93/bltins/trap.c:
- Use the normal method of parsing options with optget to fix the
  suspend builtin's test failure.

src/cmd/ksh93/tests/builtins.sh:
- Add the ksh2020 regression test for getting the usage information
  of each builtin. Enable all /opt/ast/bin builtins in a subshell
  since those should be tested as well (aside from getconf and uname
  because those builtins fallback to the real commands on error).
2020-07-26 02:18:49 +01:00
Martijn Dekker
bc8b36faba whence -a/type -a: report both function and built-in by same name
'whence -a' is documented to list all possible interpretations of a
command, but failed to list a built-in command if a shell function
by the same name exists or is marked undefined using 'autoload'.

src/cmd/ksh93/bltins/whence.c: whence():
- Refactor and separate the code for reporting functions and
  built-in commands so that both can be reported for one name.

src/cmd/ksh93/data/builtins.c: sh_optwhence[]:
- Correct 'whence --man' to document that:
  * 'type' is equivalent to 'whence -v'
  * '-a' output is like '-v'

src/cmd/ksh93/tests/builtins.sh:
- Test 'whence -a' with these combinations:
  * a function, built-in and external command
  * an undefined/autoload function, built-in and external command

Fixes https://github.com/ksh93/ksh/issues/83
2020-07-20 21:16:24 +01:00
Martijn Dekker
01c25cb14b whence -a: fix spurious 'undefined function' message
$ ksh -c 'whence -a printf'
	printf is a shell builtin
	printf is /usr/bin/printf
	printf is an undefined function

The third line should not appear.

src/cmd/ksh93/bltins/whence.c:
- Remove faulty extra check for undefined (= autoload) functions.
  This was already handled earlier, on lines 192-193.

src/cmd/ksh93/tests/builtins.sh:
- Add regression test.
- For previous 'whence -a' test, don't bother with shell function.

Fixes https://github.com/ksh93/ksh/issues/26
2020-07-20 17:03:04 +01:00
Johnothan King
bd88cc7f4f
Fix two crashes related to kshdb (#82)
This commit fixes two different crashes related to kshdb:
- When redirect is given an invalid file descriptor, a segfault
  no longer occurs. Reproducer:
  $ ksh -c 'redirect 9>&200000000000'

- Fix a crash due to free(3) being used on an invalid pointer.
  This can be reproduced with kshdb (commands from att/ast#582):
  $ git clone https://github.com/rocky/kshdb.git
  $ cd kshdb
  $ ksh autogen.sh
  $ echo "print hi there" > $HOME/.kshdbrc
  $ ./kshdb -L . test/example/dbg-test1.sh

src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- The string pointed to by shp->st.filename must be able to be
  freed from memory with free(3), so duplicate the string with
  strdup(3).

src/cmd/ksh93/sh/io.c: sh_redirect():
- Show an error message when a file descriptor is invalid to
  fix a memory fault.
2020-07-19 23:42:12 +01:00
Martijn Dekker
36f55f1f85 bltins/whence.c: Revert accidentally included test (re: 3613da42)
Some temp debug code that tests a possible fix for #26 accidentally
snuck in to a completely unrelated commit. Sorry about that.
2020-07-19 06:42:53 +01:00