Fix bad job control msg on trapped SIGINT and set -b (re: fc655f1)

When running an external command while trapping Ctrl+C via SIGINT,
and set -b is on, then a spurious Done job control message is
printed. No background job was executed.

    $ trap 'ls' INT
    $ set -b
    $ <Ctrl+C>[file listing follows]

    [1] +  Done                    set -b

In jobs.c (487-493), job_reap() calls job_list() to list a running
or completed background job, passing the JOB_NFLAG bit to only
print jobs with the P_NOTIFY flag. But the 'ls' in the trap is not
a background job. So it is getting the P_NOTIFY flag by mistake.

In fact all processes get the P_NOTIFY flag by default when they
terminate. Somehow the shell normally does not follow a code path
that calls job_list() for foreground processes, but does when
running one from a trap. I have not yet figured out how that works.

What I do know is that there is no reason why non-background
processes should ever have the P_NOTIFY flag set on termination,
because those should never print any 'Done' messages. And we seem
to have a handy P_BG flag that is set for background processes; we
can check for this before setting P_NOTIFY. The only thing is that
flag is only compiled in if SHOPT_BGX is enabled, as it was added
to support that functionality.

For some reason I am unable to reproduce the bug in a pty session,
so there is no pty.sh regression test.

src/cmd/ksh93/sh/jobs.c:
- Rename misleadingly named P_FG flag to P_MOVED2FG; this flag is
  not set for all foreground processes but only for processes moved
  to the foreground by job_switch(), called by the fg command.
- Compile in the P_BG flag even when SHOPT_BGX is not enabled. We
  need to set this flag to check for a background job.
- job_reap(): Do not set the P_NOTIFY flag for all terminated
  processes, but only for those with P_BG set.

src/cmd/ksh93/sh/xec.c: sh_fork():
- Also pass special argument 1 for background job to job_post() if
  SHOPT_BGX is not enabled. This is what gets it to set P_BG.
- Simplify 5 lines of convoluted code into 1.

Resolves: https://github.com/ksh93/ksh/issues/481

NEWS

@@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2022-07-14:
+
+- Fixed a bug that caused a spurious "Done" message on the interactive shell
+  when an external command was run as a foreground job from a SIGINT trap.
+
 2022-07-12:
 
 - The .sh.level variable can now only be changed within a DEBUG trap. When

src/cmd/ksh93/include/version.h

@@ -23,7 +23,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.0-beta.2"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2022-07-12"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2022-07-14"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/sh/jobs.c

@@ -153,10 +153,8 @@ struct back_save
 #define P_DONE		040
 #define P_COREDUMP	0100
 #define P_DISOWN	0200
-#define P_FG		0400
+#define P_MOVED2FG	0400	/* set if the process was moved to the foreground by job_switch() */
-#if SHOPT_BGX
+#define P_BG		01000	/* set if the process is running in the background */
-#define P_BG		01000
-#endif /* SHOPT_BGX */
 
 static int		job_chksave(pid_t);
 static struct process	*job_bypid(pid_t);
@@ -405,9 +403,12 @@ int job_reap(register int sig)
 				sh_subjobcheck(pid);
 
 			pw->p_flag &= ~(P_STOPPED|P_SIGNALLED);
+			pw->p_flag |= P_DONE;
+			if(pw->p_flag&P_BG)
+				pw->p_flag |= P_NOTIFY;
 			if (WIFSIGNALED(wstat))
 			{
-				pw->p_flag |= (P_DONE|P_NOTIFY|P_SIGNALLED);
+				pw->p_flag |= P_SIGNALLED;
 				if (WTERMCORE(wstat))
 					pw->p_flag |= P_COREDUMP;
 				pw->p_exit = WTERMSIG(wstat);
@@ -424,13 +425,12 @@ int job_reap(register int sig)
 			}
 			else
 			{
-				pw->p_flag |= (P_DONE|P_NOTIFY);
 				pw->p_exit =  pw->p_exitmin;
 				if(WEXITSTATUS(wstat) > pw->p_exitmin)
 					pw->p_exit = WEXITSTATUS(wstat);
 			}
 #if SHOPT_BGX
-			if((pw->p_flag&P_DONE) && (pw->p_flag&P_BG))
+			if(pw->p_flag&P_BG)
 			{
 				job.numbjob--;
 				if(sh.st.trapcom[SIGCHLD])
@@ -786,7 +786,7 @@ static void job_reset(register struct process *pw)
 		return;
 #endif	/* SIGTSTP */
 	/* force the following tty_get() to do a tcgetattr() unless fg */
-	if(!(pw->p_flag&P_FG))
+	if(!(pw->p_flag&P_MOVED2FG))
 		tty_set(-1, 0, NIL(struct termios*));
 	if(pw && (pw->p_flag&P_SIGNALLED) && pw->p_exit!=SIGHUP)
 	{
@@ -1206,18 +1206,16 @@ void	job_clear(void)
 }
 
 /*
- * put the process <pid> on the process list and return the job number
+ * Put the process <pid> on the process list and return the job number.
- * if non-zero, <join> is the process ID of the job to join
+ * If <join> is 1, the job is marked as a background job (P_BG);
+ * otherwise, if non-zero, <join> is the process ID of the job to join.
  */
 int job_post(pid_t pid, pid_t join)
 {
 	register struct process *pw;
 	register History_t *hp = sh.hist_ptr;
-#if SHOPT_BGX
-	int val,bg=0;
-#else
 	int val;
-#endif
+	char bg = 0;
 	sh.jobenv = sh.curenv;
 	if(job.toclear)
 	{
@@ -1225,14 +1223,14 @@ int job_post(pid_t pid, pid_t join)
 		return(0);
 	}
 	job_lock();
-#if SHOPT_BGX
 	if(join==1)
 	{
 		join = 0;
-		bg = P_BG;
+		bg++;
+#if SHOPT_BGX
 		job.numbjob++;
-	}
 #endif /* SHOPT_BGX */
+	}
 	if(njob_savelist < NJOB_SAVELIST)
 		init_savelist();
 	if(pw = job_bypid(pid))
@@ -1316,15 +1314,15 @@ int job_post(pid_t pid, pid_t join)
 		else
 			pw->p_flag |= (P_DONE|P_NOTIFY);
 	}
-#if SHOPT_BGX
 	if(bg)
 	{
-		if(pw->p_flag&P_DONE)
+		if(!(pw->p_flag&P_DONE))
-			job.numbjob--;
-		else
 			pw->p_flag |= P_BG;
-	}
+#if SHOPT_BGX
+		else
+			job.numbjob--;
 #endif /* SHOPT_BGX */
+	}
 	lastpid = 0;
 	job_unlock();
 	return(pw->p_job);
@@ -1608,9 +1606,7 @@ int job_switch(register struct process *pw,int bgflag)
 	{
 		sfprintf(outfile,"[%d]\t",(int)pw->p_job);
 		sh.bckpid = pw->p_pid;
-#if SHOPT_BGX
 		pw->p_flag |= P_BG;
-#endif
 		msg = "&";
 	}
 	else
@@ -1631,10 +1627,8 @@ int job_switch(register struct process *pw,int bgflag)
 			return(1);
 		}
 		job.waitall = 1;
-		pw->p_flag |= P_FG;
+		pw->p_flag |= P_MOVED2FG;
-#if SHOPT_BGX
 		pw->p_flag &= ~P_BG;
-#endif
 		job_wait(pw->p_pid);
 		job.waitall = 0;
 	}

src/cmd/ksh93/sh/xec.c

@@ -2887,15 +2887,7 @@ pid_t _sh_fork(register pid_t parent,int flags,int *jobid)
 			sh.cpid = parent;
 		if(!postid && job.curjobid && (flags&FPOU))
 			postid = job.curpgid;
-#if SHOPT_BGX
+		myjob = job_post(parent, (!postid && (flags&(FAMP|FINT))==(FAMP|FINT)) ? 1 : postid);
-		if(!postid && (flags&(FAMP|FINT)) == (FAMP|FINT))
-			postid = 1;
-		myjob = job_post(parent,postid);
-		if(postid==1)
-			postid = 0;
-#else
-		myjob = job_post(parent,postid);
-#endif /* SHOPT_BGX */
 		if(job.waitall && (flags&FPOU))
 		{
 			if(!job.curjobid)