Skip to content

Commit

Permalink
Fix another comsub regression (rhbz#1116508) (re: 970069a)
Browse files Browse the repository at this point in the history
Another Red Hat patch of a patch. With the new comsub mechanism,
functions could sometimes return the wrong exit status when invoked
from a command substitution.

Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-fununset.patch

I have determined that the extra setexit() in the Red Hat patch,
which copies the current exit status to $?, is not needed, as the
code for running functions already sets $? on termination. I've
added extra regression tests to prove this.

    By the way, the setexit() macro is defined like this in defs.h:

            #define exitset()	(sh.savexit=sh.exitval)

    That's more evidence (see also 3654ee7) that it does not
    matter whether you address the shell's status struct via a
    pointer. That macro is used in places that use shp pointers.
    But, that aside...

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- When waiting within a command substitution for a forked process
  to end, save & restore sh.exitval (the exit status of the command
  currently being run) so that job_wait() cannot override it.

src/cmd/ksh93/tests/functions.sh:
- Add tests based in part on the reproducer from rhbz#1116508.
  • Loading branch information
McDutchie committed Sep 26, 2020
1 parent 95225e1 commit 7e6bbf8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,9 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
job.bktick_waitall = 0;
if(comsub!=1 && shp->spid)
{
int e = shp->exitval;
job_wait(shp->spid);
shp->exitval = e;
if(shp->pipepid==shp->spid)
shp->spid = 0;
shp->pipepid = 0;
Expand Down
31 changes: 31 additions & 0 deletions src/cmd/ksh93/tests/functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1275,5 +1275,36 @@ $SHELL <<- \EOF
EOF
[[ $? == 2 ]] && err_exit 'Over-shifting in a POSIX function does not terminate the script if the function call has a redirection'
# ======
# https://bugzilla.redhat.com/1116508
expect=$'13/37/1/1'
actual=$(
foo() {
print foo | read
return 13
}
out=$(foo)
print -n $?/
foo() {
print foo | read
ls /dev/null
return 37
}
out=$(foo)
print -n $?/
foo() {
print foo | ! read
}
out=$(foo)
print -n $?/
foo() {
! true
}
out=$(foo)
print -n $?
)
[[ $actual == "$expect" ]] || err_exit "wrong exit status from function invoked by command substitution" \
"(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
# ======
exit $((Errors<125?Errors:125))

0 comments on commit 7e6bbf8

Please sign in to comment.