Re: ksh: Fix compilation without job control

[Todd C. Miller]

On Fri, 05 Jan 2018 16:15:45 +0100, Jeremie Courreges-Anglas wrote:

> Cool, here's the diff.  unifdef gives me the same result on jobs.c,
> except for the indentation change in two conditionals.  ok?

OK millert@

 - todd

Re: ksh: Fix compilation without job control

[Klemens Nanni]

On Fri, Jan 05, 2018 at 04:15:45PM +0100, Jeremie Courreges-Anglas wrote:
> On Fri, Jan 05 2018, "Theo de Raadt"  wrote:
> >> On Fri, 05 Jan 2018 08:20:03 +0100, Jeremie Courreges-Anglas wrote:
> >> 
> >> > I kinda take job control in my shell for granted.  Todd, would it make
> >> > sense to just delete the #ifdefs?  I doubt that we'll want to ship a ksh
> >> > with no job control in space-constrained installers.
> >> 
> >> I don't see any reason to support building ksh without job control.
> >> I'd rather just delete the #ifdefs.
> >
> > Yep.
> 
> Cool, here's the diff.  unifdef gives me the same result on jobs.c,
> except for the indentation change in two conditionals.  ok?
I was about to send the exact same diff, thanks :)

Re: ksh: Fix compilation without job control

[Jeremie Courreges-Anglas]

On Fri, Jan 05 2018, "Theo de Raadt"  wrote:
>> On Fri, 05 Jan 2018 08:20:03 +0100, Jeremie Courreges-Anglas wrote:
>> 
>> > I kinda take job control in my shell for granted.  Todd, would it make
>> > sense to just delete the #ifdefs?  I doubt that we'll want to ship a ksh
>> > with no job control in space-constrained installers.
>> 
>> I don't see any reason to support building ksh without job control.
>> I'd rather just delete the #ifdefs.
>
> Yep.

Cool, here's the diff.  unifdef gives me the same result on jobs.c,
except for the indentation change in two conditionals.  ok?


Index: c_ksh.c
===
RCS file: /d/cvs/src/bin/ksh/c_ksh.c,v
retrieving revision 1.54
diff -u -p -r1.54 c_ksh.c
--- c_ksh.c 4 Jan 2018 19:06:16 -   1.54
+++ c_ksh.c 5 Jan 2018 15:07:03 -
@@ -1068,7 +1068,6 @@ c_jobs(char **wp)
return rv;
 }
 
-#ifdef JOBS
 int
 c_fgbg(char **wp)
 {
@@ -1092,7 +1091,6 @@ c_fgbg(char **wp)
 */
return (bg || Flag(FPOSIX)) ? 0 : rv;
 }
-#endif
 
 struct kill_info {
int num_width;
@@ -1398,10 +1396,8 @@ const struct builtin kshbuiltins [] = {
{"=typeset", c_typeset},
{"+unalias", c_unalias},
{"whence", c_whence},
-#ifdef JOBS
{"+bg", c_fgbg},
{"+fg", c_fgbg},
-#endif
 #ifdef EMACS
{"bind", c_bind},
 #endif
Index: config.h
===
RCS file: /d/cvs/src/bin/ksh/config.h,v
retrieving revision 1.16
diff -u -p -r1.16 config.h
--- config.h1 Aug 2017 14:30:05 -   1.16
+++ config.h5 Jan 2018 15:07:03 -
@@ -11,9 +11,6 @@
 #ifndef CONFIG_H
 #define CONFIG_H
 
-/* Include job control? */
-#define JOBS 1
-
 /* Include brace-expansion? */
 #define BRACE_EXPAND 1
 
Index: jobs.c
===
RCS file: /d/cvs/src/bin/ksh/jobs.c,v
retrieving revision 1.55
diff -u -p -r1.55 jobs.c
--- jobs.c  17 Mar 2016 23:33:23 -  1.55
+++ jobs.c  5 Jan 2018 15:07:03 -
@@ -86,10 +86,8 @@ struct job {
Proc*proc_list; /* process list */
Proc*last_proc; /* last process in list */
Coproc_id coproc_id;/* 0 or id of coprocess output pipe */
-#ifdef JOBS
struct termios ttystate;/* saved tty state for stopped jobs */
pid_t   saved_ttypgrp;  /* saved tty process group for stopped jobs */
-#endif /* JOBS */
 };
 
 /* Flags for j_waitj() */
@@ -127,13 +125,11 @@ static intchild_max;  /* CHILD_MAX */
 /* held_sigchld is set if sigchld occurs before a job is completely started */
 static volatile sig_atomic_t held_sigchld;
 
-#ifdef JOBS
 static struct shf  *shl_j;
 static int ttypgrp_ok; /* set if can use tty pgrps */
 static pid_t   restore_ttypgrp = -1;
 static pid_t   our_pgrp;
 static int const   tt_sigs[] = { SIGTSTP, SIGTTIN, SIGTTOU };
-#endif /* JOBS */
 
 static voidj_set_async(Job *);
 static voidj_startjob(Job *);
@@ -163,7 +159,6 @@ j_init(int mflagset)
setsig([SIGCHLD], j_sigchld,
SS_RESTORE_ORIG|SS_FORCE|SS_SHTRAP);
 
-#ifdef JOBS
if (!mflagset && Flag(FTALKING))
Flag(FMONITOR) = 1;
 
@@ -189,10 +184,8 @@ j_init(int mflagset)
/* j_change() calls tty_init() */
if (Flag(FMONITOR))
j_change();
-   else
-#endif /* JOBS */
-   if (Flag(FTALKING))
-   tty_init(true);
+   else if (Flag(FTALKING))
+   tty_init(true);
 }
 
 /* suspend the shell */
@@ -267,21 +260,18 @@ j_exit(void)
kill_job(j, SIGHUP);
else
killpg(j->pgrp, SIGHUP);
-#ifdef JOBS
if (j->state == PSTOPPED) {
if (j->pgrp == 0)
kill_job(j, SIGCONT);
else
killpg(j->pgrp, SIGCONT);
}
-#endif /* JOBS */
}
}
if (killed)
sleep(1);
j_notify();
 
-#ifdef JOBS
if (kshpid == procpid && restore_ttypgrp >= 0) {
/* Need to restore the tty pgrp to what it was when the
 * shell started up, so that the process that started us
@@ -297,10 +287,8 @@ j_exit(void)
Flag(FMONITOR) = 0;
j_change();
}
-#endif /* JOBS */
 }
 
-#ifdef JOBS
 /* turn job control on or off according to Flag(FMONITOR) */
 void
 j_change(void)
@@ -389,7 +377,6 @@ j_change(void)
tty_close();
}
 }
-#endif /* JOBS */
 
 /* execute tree in child subprocess */
 int
@@ -472,7 +459,6 @@ exchild(struct op *t, int flags, volatil
else
p->pid = i;
 
-#ifdef JOBS
/* job 

Re: ksh: Fix compilation without job control

[Theo de Raadt]

> On Fri, 05 Jan 2018 08:20:03 +0100, Jeremie Courreges-Anglas wrote:
> 
> > I kinda take job control in my shell for granted.  Todd, would it make
> > sense to just delete the #ifdefs?  I doubt that we'll want to ship a ksh
> > with no job control in space-constrained installers.
> 
> I don't see any reason to support building ksh without job control.
> I'd rather just delete the #ifdefs.

Yep.

Re: ksh: Fix compilation without job control

[Todd C. Miller]

On Fri, 05 Jan 2018 08:20:03 +0100, Jeremie Courreges-Anglas wrote:

> I kinda take job control in my shell for granted.  Todd, would it make
> sense to just delete the #ifdefs?  I doubt that we'll want to ship a ksh
> with no job control in space-constrained installers.

I don't see any reason to support building ksh without job control.
I'd rather just delete the #ifdefs.

 - todd

Re: ksh: Fix compilation without job control

[Jeremie Courreges-Anglas]

On Thu, Jan 04 2018, Klemens Nanni  wrote:
> A few guards are missing, with this diff ksh compiles cleanly with JOBS
> undefined.
>
> Found while looking for undesired changes after working on job control.
>
> Feedback?

Looks good, ok jca@

I kinda take job control in my shell for granted.  Todd, would it make
sense to just delete the #ifdefs?  I doubt that we'll want to ship a ksh
with no job control in space-constrained installers.

> diff --git a/bin/ksh/jobs.c b/bin/ksh/jobs.c
> index 53858a37d26..7e29c2f38b8 100644
> --- a/bin/ksh/jobs.c
> +++ b/bin/ksh/jobs.c
> @@ -201,6 +201,7 @@ j_suspend(void)
>  {
>   struct sigaction sa, osa;
>  
> +#ifdef JOBS
>   /* Restore tty and pgrp. */
>   if (ttypgrp_ok) {
>   tcsetattr(tty_fd, TCSADRAIN, _state);
> @@ -218,6 +219,7 @@ j_suspend(void)
>   }
>   }
>   }
> +#endif /* JOBS */
>  
>   /* Suspend the shell. */
>   memset(, 0, sizeof(sa));
> @@ -228,6 +230,7 @@ j_suspend(void)
>  
>   /* Back from suspend, reset signals, pgrp and tty. */
>   sigaction(SIGTSTP, , NULL);
> +#ifdef JOBS
>   if (ttypgrp_ok) {
>   if (restore_ttypgrp >= 0) {
>   if (setpgid(0, kshpid) < 0) {
> @@ -246,6 +249,7 @@ j_suspend(void)
>   }
>   tty_init(true);
>   }
> +#endif /* JOBS */
>  }
>  
>  /* job cleanup before shell exit */
> @@ -1050,10 +1054,10 @@ j_waitj(Job *j,
>   j->flags &= ~(JF_WAITING|JF_W_ASYNCNOTIFY);
>  
>   if (j->flags & JF_FG) {
> - int status;
> -
>   j->flags &= ~JF_FG;
>  #ifdef JOBS
> + int status;
> +
>   if (Flag(FMONITOR) && ttypgrp_ok && j->pgrp) {
>   /*
>* Save the tty's current pgrp so it can be restored
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE