Skip to content

Commit

Permalink
sh_exec(): remove a vfork(2) use for the >; redirection
Browse files Browse the repository at this point in the history
vfork has been removed from the POSIX standard and macOS gives a
deprecation warning for it.

Additionally, the old (2004) POSIX spec for it states:
https://pubs.opengroup.org/onlinepubs/009696799/functions/vfork.html
| The vfork() function shall be equivalent to fork(), except that
| the behavior is undefined if the process created by vfork()
| either modifies any data other than a variable of type pid_t used
| to store the return value from vfork(), or returns from the
| function in which vfork() was called, or calls any other function
| before successfully calling _exit() or one of the exec family of
| functions.

According to that spec, this vfork(2) usage was very much engaged
in undefined behaviour, as the first thing it does after vforking
is call _sh_fork(), and it never calls an exec function.

The lib_vfork feature test in src/lib/libast/features/lib does try
to verify that vfork works and does not merely exist, but it does
not attempt to run anything substantial in the child process
(besides, 'undefined behaviour' does not mean the test would
necessarily fail). That test may be good enough for the spawnveg(3)
vfork fallback, but it is inherently unreliable for determining if
we can run anything substantial in the child.

(This if(rewrite) code block is used when the >; redirection
operator (IOREWRITE) is used with an external command. It won't
work without the extra (v)fork, as the external command will be
replacing the ksh child process but ksh needs to decide if the
command succeeds before rewriting the output from a temp file.)
  • Loading branch information
McDutchie committed Apr 5, 2023
1 parent 45b1808 commit b810e2d
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/shnodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ struct arithnod
#define IOVNM 0x10000 /* iovname field is non-zero */
#define IOLSEEK 0x20000 /* seek operators <# or ># */
#define IOARITH 0x40000 /* arithmetic seek <# ((expr)) */
#define IOREWRITE 0x80000 /* arithmetic seek <# ((expr)) */
#define IOREWRITE 0x80000 /* rewrite/truncate upon command success: >;word <>;word */
#define IOCOPY IOCLOB /* copy skipped lines onto standard output */
#define IOPROCSUB IOARITH /* process substitution redirection */

Expand Down
8 changes: 1 addition & 7 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@
# include <vmalloc.h>
#endif

#if _lib_vfork
# include <ast_vfork.h>
#else
# define vfork() fork()
#endif

#if _lib_getrusage && !defined(RUSAGE_SELF)
# include <sys/resource.h>
#endif
Expand Down Expand Up @@ -1633,7 +1627,7 @@ int sh_exec(const Shnode_t *t, int flags)
if(rewrite)
{
job_lock();
while((parent = vfork()) < 0)
while((parent = fork()) < 0)
_sh_fork(parent, 0, NULL);
if(parent)
{
Expand Down

0 comments on commit b810e2d

Please sign in to comment.