Skip to content

Commit

Permalink
Don't turn off SH_PROFILE in subshells (re: 3785a06, 97353a0)
Browse files Browse the repository at this point in the history
Some of the behaviours changes effectuated by the SH_PROFILE flag
(on while executing profile scripts like .kshrc) *should* remain in
effect in subshells. These include:

* $0 does not change within ksh functions (sh/macro.c:2856)
* dot scripts are parsed with SH_FUNEVAL, i.e., line by line and
  not all at once (bltins/misc.c:325)
* background job commands for the jobs list should not be taken
  from the history file (sh/jobs.c:1165)
* syntax error messages do not add an extra line number indication
  (sh/lex.c:2118)

The two referenced commits potentially introduced incorrect
behaviour for the above. On the other hand, the behaviours that
*should* change in subshells, even with SH_PROFILE on, and thus
were fixed by the referenced commits, are these:

* a background job from a subshell should not print a job number
  (sh/xec.c:1560)
* 'return' outside of functions in a subshell should still act like
  'exit' (bltins/cflow.c:47)

So the latter two cases need to be fixed separately instead,
i.e., without turning off the SH_PROFILE flag.

src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/args.c:
- Revert changes from the referenced commits: do not turn off the
  SH_PROFILE flag in subshells, or before forking to execute a
  process substitution.

src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/sh/args.c,
src/cmd/ksh93/sh/xec.c:
- Add SH_PROCSUB state flag.
- Set it in procsubst() before calling sh_exec() to run a process
  substitution. This is done before forking, so a sh.realsubshell
  check is not suitable for this; hence the state flag.
- In sh_exec(), do not print job number if sh_isstate(SH_PROCSUB).
  This reimplements the bugfix from the first referenced commit.

src/cmd/ksh93/bltins/cflow.c:
- When determining whether return should act like exit, do not
  honour SH_PROFILE if we're in a subshell (virtual or forked).
  • Loading branch information
McDutchie committed Aug 23, 2024
1 parent abeb5ea commit 3c222cc
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/cmd/ksh93/bltins/cflow.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
int b_return(int n, char *argv[],Shbltin_t *context)
{
/* 'return' outside of function, dotscript and profile behaves like 'exit' */
char do_exit = **argv=='e' || sh.fn_depth==0 && sh.dot_depth==0 && !sh_isstate(SH_PROFILE);
const int do_exit = **argv=='e' || sh.fn_depth==0 && sh.dot_depth==0 && (!sh_isstate(SH_PROFILE) || sh.realsubshell);
NOT_USED(context);
while((n = optget(argv, **argv=='e' ? sh_optexit : sh_optreturn))) switch(n)
{
Expand Down
1 change: 1 addition & 0 deletions src/cmd/ksh93/include/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ typedef union Shnode_u Shnode_t;
#define SH_XARG 21 /* set while in xarg (command -x) mode */
#define SH_NOTILDEXP 22 /* set to disable tilde expansion */
#define SH_EXEC 23 /* set while in exec(1) */
#define SH_PROCSUB 24 /* set (also in parent shell) while executing process substitution */

/*
* Shell options (set -o). Used with sh_isoption(), sh_onoption(), sh_offoption().
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.0.11-beta" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2024-08-22" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-08-23" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/args.c
Original file line number Diff line number Diff line change
Expand Up @@ -761,14 +761,14 @@ struct argnod *sh_argprocsub(struct argnod *argp)
/* turn off job control */
sh_offstate(SH_INTERACTIVE);
sh_offstate(SH_MONITOR);
sh_offstate(SH_PROFILE);
job.jobcontrol = 0;
/* run the process substitution */
sh.subshell = 0;
if(fd)
sh.inpipe = pv;
else
sh.outpipe = pv;
sh_onstate(SH_PROCSUB);
sh_exec((Shnode_t*)argp->argchn.ap,(int)sh_isstate(SH_ERREXIT));
/* restore the previous state */
sh.subshell = savesubshell;
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/jobs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1162,7 +1162,7 @@ int job_post(pid_t pid, pid_t join)
pw->p_pid,pw->p_pgrp,job.savesig,join);
sfsync(sfstderr);
#endif /* DEBUG */
if(hp && !sh_isstate(SH_PROFILE))
if(hp && !sh_isstate(SH_PROFILE) && !sh.realsubshell)
pw->p_name=hist_tell(sh.hist_ptr,(int)hp->histind-1);
else
pw->p_name = -1;
Expand Down
1 change: 0 additions & 1 deletion src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,6 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub)
else
sh_subfork();
}
sh_offstate(SH_PROFILE);
sh_exec(t,flags);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -1505,7 +1505,7 @@ int sh_exec(const Shnode_t *t, int flags)
sigrelease(SIGINT);
}
/* print job number */
if(type&FAMP && (sh_isstate(SH_PROFILE) || sh_isstate(SH_INTERACTIVE)))
if(type&FAMP && (sh_isstate(SH_INTERACTIVE) || sh_isstate(SH_PROFILE)) && !sh_isstate(SH_PROCSUB) && !sh.realsubshell)
sfprintf(sfstderr,"[%d]\t%d\n",jobid,parent);
break;
}
Expand Down
19 changes: 15 additions & 4 deletions src/cmd/ksh93/tests/options.sh
Original file line number Diff line number Diff line change
Expand Up @@ -531,10 +531,21 @@ do if { date | true;} ; true
done
(( (SECONDS-t1) > .5 )) && err_exit 'pipefail should not wait for background processes'

# process source files from profiles as profile files
print '. ./dotfile' > envfile
print $'alias print=:\nprint foobar' > dotfile
[[ $(ENV=/.$PWD/envfile $SHELL -i -c : 2>/dev/null) == foobar ]] && err_exit 'files source from profile does not process aliases correctly'
# ======
print $'v=$(. ./dotfile)\n(. ./dotfile)\ncat <(. ./dotfile)\n. ./dotfile' > envfile
# dot scripts sourced from profile files are parsed line by line, so that aliases take effect on the next line in the same file
print $'alias print=:\nprint fail:subshell==${.sh.subshell} >&2' > dotfile
got=$(ENV=/.$PWD/envfile "$SHELL" -i -c : 2>&1)
exp=''
[[ $got == "$exp" ]] || err_exit 'dot script sourced from profile does not process aliases correctly' \
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
# $0 does not change in ksh functions within profile scripts
print 'function BAD { echo $0; }; BAD >&2' > dotfile
got=$(ENV=/.$PWD/envfile "$SHELL" -i -c : 2>&1)
exp=$SHELL$'\n'$SHELL$'\n'$SHELL$'\n'$SHELL
[[ $got == "$exp" ]] || err_exit '$0 in ksh function in profile script not correct' \
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"


# ======
if [[ -o ?posix ]]; then
Expand Down

0 comments on commit 3c222cc

Please sign in to comment.