From d81e9f5093b222369d510d0b1c0587a411e4c83e Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 28 Oct 2016 15:43:50 +0200 Subject: ash: fix interactive "command eval STRING" exiting on errors. This bug is also present in current dash Signed-off-by: Denys Vlasenko --- shell/ash.c | 25 ++++++++++++++++++++++++- shell/ash_test/ash-vars/readonly1.right | 2 ++ shell/ash_test/ash-vars/readonly1.tests | 7 +++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 shell/ash_test/ash-vars/readonly1.right create mode 100755 shell/ash_test/ash-vars/readonly1.tests diff --git a/shell/ash.c b/shell/ash.c index 1ef02b8be..fe112453c 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -2180,6 +2180,7 @@ setvareq(char *s, int flags) if (flags & VNOSAVE) free(s); n = vp->var_text; + exitstatus = 1; ash_msg_and_raise_error("%.*s: is read only", strchrnul(n, '=') - n, n); } @@ -9599,7 +9600,7 @@ evalcommand(union node *cmd, int flags) if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) { if (exception_type == EXERROR && spclbltin <= 0) { FORCE_INT_ON; - break; + goto readstatus; } raise: longjmp(exception_handler->loc, 1); @@ -12280,6 +12281,10 @@ expandstr(const char *ps) static int evalstring(char *s, int flags) { + struct jmploc *volatile savehandler = exception_handler; + struct jmploc jmploc; + int ex; + union node *n; struct stackmark smark; int status; @@ -12289,6 +12294,19 @@ evalstring(char *s, int flags) setstackmark(&smark); status = 0; + /* On exception inside execution loop, we must popfile(). + * Try interactively: + * readonly a=a + * command eval "a=b" # throws "is read only" error + * "command BLTIN" is not supposed to abort (even in non-interactive use). + * But if we skip popfile(), we hit EOF in eval's string, and exit. + */ + savehandler = exception_handler; + exception_handler = &jmploc; + ex = setjmp(jmploc.loc); + if (ex) + goto out; + while ((n = parsecmd(0)) != NODE_EOF) { int i; @@ -12299,10 +12317,15 @@ evalstring(char *s, int flags) if (evalskip) break; } + out: popstackmark(&smark); popfile(); stunalloc(s); + exception_handler = savehandler; + if (ex) + longjmp(exception_handler->loc, ex); + return status; } diff --git a/shell/ash_test/ash-vars/readonly1.right b/shell/ash_test/ash-vars/readonly1.right new file mode 100644 index 000000000..2b363e325 --- /dev/null +++ b/shell/ash_test/ash-vars/readonly1.right @@ -0,0 +1,2 @@ +One:1 +One:1 diff --git a/shell/ash_test/ash-vars/readonly1.tests b/shell/ash_test/ash-vars/readonly1.tests new file mode 100755 index 000000000..81b461f5f --- /dev/null +++ b/shell/ash_test/ash-vars/readonly1.tests @@ -0,0 +1,7 @@ +readonly bla=123 +# Bare "eval bla=123" should abort ("eval" is a special builtin): +(eval bla=123 2>/dev/null; echo BUG) +echo One:$? +# "command BLTIN" disables "special-ness", should not abort: +command eval bla=123 2>/dev/null +echo One:$? -- cgit v1.2.3-55-g6feb From 8f7b0248adca9a88351fd7f3dd208775242f3fe6 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 28 Oct 2016 17:16:11 +0200 Subject: ash: use pause(), not sigsuspend(), in wait builtin Same effect, smaller code function old new delta dowait 463 374 -89 Signed-off-by: Denys Vlasenko --- shell/ash.c | 57 +++++++++++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index fe112453c..fc1b5d927 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -296,8 +296,7 @@ struct globals_misc { volatile int suppress_int; /* counter */ volatile /*sig_atomic_t*/ smallint pending_int; /* 1 = got SIGINT */ volatile /*sig_atomic_t*/ smallint got_sigchld; /* 1 = got SIGCHLD */ - /* last pending signal */ - volatile /*sig_atomic_t*/ smallint pending_sig; + volatile /*sig_atomic_t*/ smallint pending_sig; /* last pending signal */ smallint exception_type; /* kind of exception (0..5) */ /* exceptions */ #define EXINT 0 /* SIGINT received */ @@ -3515,11 +3514,6 @@ setsignal(int signo) #define CUR_RUNNING 1 #define CUR_STOPPED 0 -/* mode flags for dowait */ -#define DOWAIT_NONBLOCK 0 -#define DOWAIT_BLOCK 1 -#define DOWAIT_BLOCK_OR_SIG 2 - #if JOBS /* pgrp of shell on invocation */ static int initialpgrp; //references:2 @@ -3940,24 +3934,30 @@ sprint_status48(char *s, int status, int sigonly) } static int -wait_block_or_sig(int *status, int wait_flags) +wait_block_or_sig(int *status) { - sigset_t mask; int pid; do { /* Poll all children for changes in their state */ got_sigchld = 0; - pid = waitpid(-1, status, wait_flags | WNOHANG); + /* if job control is active, accept stopped processes too */ + pid = waitpid(-1, status, doing_jobctl ? (WNOHANG|WUNTRACED) : WNOHANG); if (pid != 0) - break; /* Error (e.g. EINTR) or pid */ + break; /* Error (e.g. EINTR, ECHILD) or pid */ - /* No child is ready. Sleep until interesting signal is received */ + /* Children exist, but none are ready. Sleep until interesting signal */ +#if 0 /* dash does this */ + sigset_t mask; sigfillset(&mask); sigprocmask(SIG_SETMASK, &mask, &mask); while (!got_sigchld && !pending_sig) sigsuspend(&mask); sigprocmask(SIG_SETMASK, &mask, NULL); +#else + while (!got_sigchld && !pending_sig) + pause(); +#endif /* If it was SIGCHLD, poll children again */ } while (got_sigchld); @@ -3965,11 +3965,13 @@ wait_block_or_sig(int *status, int wait_flags) return pid; } +#define DOWAIT_NONBLOCK 0 +#define DOWAIT_BLOCK 1 +#define DOWAIT_BLOCK_OR_SIG 2 static int dowait(int block, struct job *job) { - int wait_flags; int pid; int status; struct job *jp; @@ -3977,13 +3979,6 @@ dowait(int block, struct job *job) TRACE(("dowait(0x%x) called\n", block)); - wait_flags = 0; - if (block == DOWAIT_NONBLOCK) - wait_flags = WNOHANG; - /* If job control is compiled in, we accept stopped processes too. */ - if (doing_jobctl) - wait_flags |= WUNTRACED; - /* It's wrong to call waitpid() outside of INT_OFF region: * signal can arrive just after syscall return and handler can * longjmp away, losing stop/exit notification processing. @@ -3994,17 +3989,27 @@ dowait(int block, struct job *job) * in INT_OFF region: "wait" needs to wait for any running job * to change state, but should exit on any trap too. * In INT_OFF region, a signal just before syscall entry can set - * pending_sig valiables, but we can't check them, and we would + * pending_sig variables, but we can't check them, and we would * either enter a sleeping waitpid() (BUG), or need to busy-loop. + * * Because of this, we run inside INT_OFF, but use a special routine - * which combines waitpid() and sigsuspend(). + * which combines waitpid() and pause(). + * This is the reason why we need to have a handler for SIGCHLD: + * SIG_DFL handler does not wake pause(). */ INT_OFF; - if (block == DOWAIT_BLOCK_OR_SIG) - pid = wait_block_or_sig(&status, wait_flags); - else - /* NB: _not_ safe_waitpid, we need to detect EINTR. */ + if (block == DOWAIT_BLOCK_OR_SIG) { + pid = wait_block_or_sig(&status); + } else { + int wait_flags = 0; + if (block == DOWAIT_NONBLOCK) + wait_flags = WNOHANG; + /* if job control is active, accept stopped processes too */ + if (doing_jobctl) + wait_flags |= WUNTRACED; + /* NB: _not_ safe_waitpid, we need to detect EINTR */ pid = waitpid(-1, &status, wait_flags); + } TRACE(("wait returns pid=%d, status=0x%x, errno=%d(%s)\n", pid, status, errno, strerror(errno))); if (pid <= 0) -- cgit v1.2.3-55-g6feb From 7e6753609d102b68a625072fb1660065246a54e2 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 28 Oct 2016 21:57:31 +0200 Subject: hush: fix "wait PID" It was not properly interruptible, and did not update job status (the exited processes were still thought of as running). function old new delta process_wait_result - 453 +453 wait_for_child_or_signal - 199 +199 run_list 996 1002 +6 checkjobs_and_fg_shell 41 43 +2 builtin_wait 328 215 -113 checkjobs 516 142 -374 ------------------------------------------------------------------------------ (add/remove: 2/0 grow/shrink: 2/2 up/down: 660/-487) Total: 173 bytes Signed-off-by: Denys Vlasenko --- shell/hush.c | 404 +++++++++++++++++++--------------- shell/hush_test/hush-misc/wait1.right | 2 + shell/hush_test/hush-misc/wait1.tests | 3 + shell/hush_test/hush-misc/wait2.right | 2 + shell/hush_test/hush-misc/wait2.tests | 4 + shell/hush_test/hush-misc/wait3.right | 2 + shell/hush_test/hush-misc/wait3.tests | 3 + 7 files changed, 246 insertions(+), 174 deletions(-) create mode 100644 shell/hush_test/hush-misc/wait1.right create mode 100755 shell/hush_test/hush-misc/wait1.tests create mode 100644 shell/hush_test/hush-misc/wait2.right create mode 100755 shell/hush_test/hush-misc/wait2.tests create mode 100644 shell/hush_test/hush-misc/wait3.right create mode 100755 shell/hush_test/hush-misc/wait3.tests diff --git a/shell/hush.c b/shell/hush.c index c80429d5c..1f8a3e607 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -44,6 +44,8 @@ * special variables (done: PWD, PPID, RANDOM) * tilde expansion * aliases + * kill %jobspec + * wait %jobspec * follow IFS rules more precisely, including update semantics * builtins mandated by standards we don't support: * [un]alias, command, fc, getopts, newgrp, readonly, times @@ -7041,16 +7043,134 @@ static void delete_finished_bg_job(struct pipe *pi) } #endif /* JOB */ -/* Check to see if any processes have exited -- if they - * have, figure out why and see if a job has completed */ -static int checkjobs(struct pipe *fg_pipe) +static int process_wait_result(struct pipe *fg_pipe, pid_t childpid, int status) { - int attributes; - int status; #if ENABLE_HUSH_JOB struct pipe *pi; #endif - pid_t childpid; + int i, dead; + + dead = WIFEXITED(status) || WIFSIGNALED(status); + +#if DEBUG_JOBS + if (WIFSTOPPED(status)) + debug_printf_jobs("pid %d stopped by sig %d (exitcode %d)\n", + childpid, WSTOPSIG(status), WEXITSTATUS(status)); + if (WIFSIGNALED(status)) + debug_printf_jobs("pid %d killed by sig %d (exitcode %d)\n", + childpid, WTERMSIG(status), WEXITSTATUS(status)); + if (WIFEXITED(status)) + debug_printf_jobs("pid %d exited, exitcode %d\n", + childpid, WEXITSTATUS(status)); +#endif + /* Were we asked to wait for a fg pipe? */ + if (fg_pipe) { + i = fg_pipe->num_cmds; + while (--i >= 0) { + debug_printf_jobs("check pid %d\n", fg_pipe->cmds[i].pid); + if (fg_pipe->cmds[i].pid != childpid) + continue; + if (dead) { + int ex; + fg_pipe->cmds[i].pid = 0; + fg_pipe->alive_cmds--; + ex = WEXITSTATUS(status); + /* bash prints killer signal's name for *last* + * process in pipe (prints just newline for SIGINT/SIGPIPE). + * Mimic this. Example: "sleep 5" + (^\ or kill -QUIT) + */ + if (WIFSIGNALED(status)) { + int sig = WTERMSIG(status); + if (i == fg_pipe->num_cmds-1) + /* TODO: use strsignal() instead for bash compat? but that's bloat... */ + puts(sig == SIGINT || sig == SIGPIPE ? "" : get_signame(sig)); + /* TODO: if (WCOREDUMP(status)) + " (core dumped)"; */ + /* TODO: MIPS has 128 sigs (1..128), what if sig==128 here? + * Maybe we need to use sig | 128? */ + ex = sig + 128; + } + fg_pipe->cmds[i].cmd_exitcode = ex; + } else { + fg_pipe->stopped_cmds++; + } + debug_printf_jobs("fg_pipe: alive_cmds %d stopped_cmds %d\n", + fg_pipe->alive_cmds, fg_pipe->stopped_cmds); + if (fg_pipe->alive_cmds == fg_pipe->stopped_cmds) { + /* All processes in fg pipe have exited or stopped */ + int rcode = 0; + i = fg_pipe->num_cmds; + while (--i >= 0) { + rcode = fg_pipe->cmds[i].cmd_exitcode; + /* usually last process gives overall exitstatus, + * but with "set -o pipefail", last *failed* process does */ + if (G.o_opt[OPT_O_PIPEFAIL] == 0 || rcode != 0) + break; + } + IF_HAS_KEYWORDS(if (fg_pipe->pi_inverted) rcode = !rcode;) +/* Note: *non-interactive* bash does not continue if all processes in fg pipe + * are stopped. Testcase: "cat | cat" in a script (not on command line!) + * and "killall -STOP cat" */ + if (G_interactive_fd) { +#if ENABLE_HUSH_JOB + if (fg_pipe->alive_cmds != 0) + insert_bg_job(fg_pipe); +#endif + return rcode; + } + if (fg_pipe->alive_cmds == 0) + return rcode; + } + /* There are still running processes in the fg_pipe */ + return -1; + } + /* It wasnt in fg_pipe, look for process in bg pipes */ + } + +#if ENABLE_HUSH_JOB + /* We were asked to wait for bg or orphaned children */ + /* No need to remember exitcode in this case */ + for (pi = G.job_list; pi; pi = pi->next) { + for (i = 0; i < pi->num_cmds; i++) { + if (pi->cmds[i].pid == childpid) + goto found_pi_and_prognum; + } + } + /* Happens when shell is used as init process (init=/bin/sh) */ + debug_printf("checkjobs: pid %d was not in our list!\n", childpid); + return -1; /* this wasn't a process from fg_pipe */ + + found_pi_and_prognum: + if (dead) { + /* child exited */ + pi->cmds[i].pid = 0; + pi->cmds[i].cmd_exitcode = WEXITSTATUS(status); + if (WIFSIGNALED(status)) + pi->cmds[i].cmd_exitcode = 128 + WTERMSIG(status); + pi->alive_cmds--; + if (!pi->alive_cmds) { + if (G_interactive_fd) + printf(JOB_STATUS_FORMAT, pi->jobid, + "Done", pi->cmdtext); + delete_finished_bg_job(pi); + } + } else { + /* child stopped */ + pi->stopped_cmds++; + } +#endif + return -1; /* this wasn't a process from fg_pipe */ +} + +/* Check to see if any processes have exited -- if they have, + * figure out why and see if a job has completed. + * Alternatively (fg_pipe == NULL, waitfor_pid != 0), + * wait for a specific pid to complete, return exitcode+1 + * (this allows to distinguish zero as "no children exited" result). + */ +static int checkjobs(struct pipe *fg_pipe, pid_t waitfor_pid) +{ + int attributes; + int status; int rcode = 0; debug_printf_jobs("checkjobs %p\n", fg_pipe); @@ -7087,12 +7207,10 @@ static int checkjobs(struct pipe *fg_pipe) * 1 <========== bg pipe is not fully done, but exitcode is already known! * [hush 1.14.0: yes we do it right] */ - wait_more: while (1) { - int i; - int dead; - + pid_t childpid; #if ENABLE_HUSH_FAST + int i; i = G.count_SIGCHLD; #endif childpid = waitpid(-1, &status, attributes); @@ -7106,112 +7224,24 @@ static int checkjobs(struct pipe *fg_pipe) //bb_error_msg("[%d] checkjobs: waitpid returned <= 0, G.count_SIGCHLD:%d G.handled_SIGCHLD:%d", getpid(), G.count_SIGCHLD, G.handled_SIGCHLD); } #endif + /* ECHILD (no children), or 0 (no change in children status) */ + rcode = childpid; break; } - dead = WIFEXITED(status) || WIFSIGNALED(status); - -#if DEBUG_JOBS - if (WIFSTOPPED(status)) - debug_printf_jobs("pid %d stopped by sig %d (exitcode %d)\n", - childpid, WSTOPSIG(status), WEXITSTATUS(status)); - if (WIFSIGNALED(status)) - debug_printf_jobs("pid %d killed by sig %d (exitcode %d)\n", - childpid, WTERMSIG(status), WEXITSTATUS(status)); - if (WIFEXITED(status)) - debug_printf_jobs("pid %d exited, exitcode %d\n", - childpid, WEXITSTATUS(status)); -#endif - /* Were we asked to wait for fg pipe? */ - if (fg_pipe) { - i = fg_pipe->num_cmds; - while (--i >= 0) { - debug_printf_jobs("check pid %d\n", fg_pipe->cmds[i].pid); - if (fg_pipe->cmds[i].pid != childpid) - continue; - if (dead) { - int ex; - fg_pipe->cmds[i].pid = 0; - fg_pipe->alive_cmds--; - ex = WEXITSTATUS(status); - /* bash prints killer signal's name for *last* - * process in pipe (prints just newline for SIGINT/SIGPIPE). - * Mimic this. Example: "sleep 5" + (^\ or kill -QUIT) - */ - if (WIFSIGNALED(status)) { - int sig = WTERMSIG(status); - if (i == fg_pipe->num_cmds-1) - /* TODO: use strsignal() instead for bash compat? but that's bloat... */ - puts(sig == SIGINT || sig == SIGPIPE ? "" : get_signame(sig)); - /* TODO: if (WCOREDUMP(status)) + " (core dumped)"; */ - /* TODO: MIPS has 128 sigs (1..128), what if sig==128 here? - * Maybe we need to use sig | 128? */ - ex = sig + 128; - } - fg_pipe->cmds[i].cmd_exitcode = ex; - } else { - fg_pipe->stopped_cmds++; - } - debug_printf_jobs("fg_pipe: alive_cmds %d stopped_cmds %d\n", - fg_pipe->alive_cmds, fg_pipe->stopped_cmds); - if (fg_pipe->alive_cmds == fg_pipe->stopped_cmds) { - /* All processes in fg pipe have exited or stopped */ - i = fg_pipe->num_cmds; - while (--i >= 0) { - rcode = fg_pipe->cmds[i].cmd_exitcode; - /* usually last process gives overall exitstatus, - * but with "set -o pipefail", last *failed* process does */ - if (G.o_opt[OPT_O_PIPEFAIL] == 0 || rcode != 0) - break; - } - IF_HAS_KEYWORDS(if (fg_pipe->pi_inverted) rcode = !rcode;) -/* Note: *non-interactive* bash does not continue if all processes in fg pipe - * are stopped. Testcase: "cat | cat" in a script (not on command line!) - * and "killall -STOP cat" */ - if (G_interactive_fd) { -#if ENABLE_HUSH_JOB - if (fg_pipe->alive_cmds != 0) - insert_bg_job(fg_pipe); -#endif - return rcode; - } - if (fg_pipe->alive_cmds == 0) - return rcode; - } - /* There are still running processes in the fg pipe */ - goto wait_more; /* do waitpid again */ - } - /* it wasnt fg_pipe, look for process in bg pipes */ - } - -#if ENABLE_HUSH_JOB - /* We asked to wait for bg or orphaned children */ - /* No need to remember exitcode in this case */ - for (pi = G.job_list; pi; pi = pi->next) { - for (i = 0; i < pi->num_cmds; i++) { - if (pi->cmds[i].pid == childpid) - goto found_pi_and_prognum; - } + rcode = process_wait_result(fg_pipe, childpid, status); + if (rcode >= 0) { + /* fg_pipe exited or stopped */ + break; } - /* Happens when shell is used as init process (init=/bin/sh) */ - debug_printf("checkjobs: pid %d was not in our list!\n", childpid); - continue; /* do waitpid again */ - - found_pi_and_prognum: - if (dead) { - /* child exited */ - pi->cmds[i].pid = 0; - pi->alive_cmds--; - if (!pi->alive_cmds) { - if (G_interactive_fd) - printf(JOB_STATUS_FORMAT, pi->jobid, - "Done", pi->cmdtext); - delete_finished_bg_job(pi); - } - } else { - /* child stopped */ - pi->stopped_cmds++; + if (childpid == waitfor_pid) { + rcode = WEXITSTATUS(status); + if (WIFSIGNALED(status)) + rcode = 128 + WTERMSIG(status); + rcode++; + break; /* "wait PID" called us, give it exitcode+1 */ } -#endif + /* This wasn't one of our processes, or */ + /* fg_pipe still has running processes, do waitpid again */ } /* while (waitpid succeeds)... */ return rcode; @@ -7221,7 +7251,7 @@ static int checkjobs(struct pipe *fg_pipe) static int checkjobs_and_fg_shell(struct pipe *fg_pipe) { pid_t p; - int rcode = checkjobs(fg_pipe); + int rcode = checkjobs(fg_pipe, 0 /*(no pid to wait for)*/); if (G_saved_tty_pgrp) { /* Job finished, move the shell to the foreground */ p = getpgrp(); /* our process group id */ @@ -7893,7 +7923,7 @@ static int run_list(struct pipe *pi) /* else: e.g. "continue 2" should *break* once, *then* continue */ } /* else: "while... do... { we are here (innermost list is not a loop!) };...done" */ if (G.depth_break_continue != 0 || fbc == BC_BREAK) { - checkjobs(NULL); + checkjobs(NULL, 0 /*(no pid to wait for)*/); break; } /* "continue": simulate end of loop */ @@ -7902,7 +7932,7 @@ static int run_list(struct pipe *pi) } #endif if (G_flag_return_in_progress == 1) { - checkjobs(NULL); + checkjobs(NULL, 0 /*(no pid to wait for)*/); break; } } else if (pi->followup == PIPE_BG) { @@ -7929,7 +7959,7 @@ static int run_list(struct pipe *pi) } else #endif { /* This one just waits for completion */ - rcode = checkjobs(pi); + rcode = checkjobs(pi, 0 /*(no pid to wait for)*/); debug_printf_exec(": checkjobs exitcode %d\n", rcode); check_and_run_traps(); } @@ -7943,7 +7973,7 @@ static int run_list(struct pipe *pi) cond_code = rcode; #endif check_jobs_and_continue: - checkjobs(NULL); + checkjobs(NULL, 0 /*(no pid to wait for)*/); dont_check_jobs_but_continue: ; #if ENABLE_HUSH_LOOPS /* Beware of "while false; true; do ..."! */ @@ -9408,9 +9438,68 @@ static int FAST_FUNC builtin_umask(char **argv) } /* http://www.opengroup.org/onlinepubs/9699919799/utilities/wait.html */ +static int wait_for_child_or_signal(pid_t waitfor_pid) +{ + int ret = 0; + for (;;) { + int sig; + sigset_t oldset, allsigs; + + /* waitpid is not interruptible by SA_RESTARTed + * signals which we use. Thus, this ugly dance: + */ + + /* Make sure possible SIGCHLD is stored in kernel's + * pending signal mask before we call waitpid. + * Or else we may race with SIGCHLD, lose it, + * and get stuck in sigwaitinfo... + */ + sigfillset(&allsigs); + sigprocmask(SIG_SETMASK, &allsigs, &oldset); + + if (!sigisemptyset(&G.pending_set)) { + /* Crap! we raced with some signal! */ + // sig = 0; + goto restore; + } + + /*errno = 0; - checkjobs does this */ + ret = checkjobs(NULL, waitfor_pid); /* waitpid(WNOHANG) inside */ + /* if ECHILD, there are no children (ret is -1 or 0) */ + /* if ret == 0, no children changed state */ + /* if ret != 0, it's exitcode+1 of exited waitfor_pid child */ + if (errno == ECHILD || ret--) { + if (ret < 0) /* if ECHILD, may need to fix */ + ret = 0; + sigprocmask(SIG_SETMASK, &oldset, NULL); + break; + } + + /* Wait for SIGCHLD or any other signal */ + //sig = sigwaitinfo(&allsigs, NULL); + /* It is vitally important for sigsuspend that SIGCHLD has non-DFL handler! */ + /* Note: sigsuspend invokes signal handler */ + sigsuspend(&oldset); + restore: + sigprocmask(SIG_SETMASK, &oldset, NULL); + + /* So, did we get a signal? */ + //if (sig > 0) + // raise(sig); /* run handler */ + sig = check_and_run_traps(); + if (sig /*&& sig != SIGCHLD - always true */) { + /* see note 2 */ + ret = 128 + sig; + break; + } + /* SIGCHLD, or no signal, or ignored one, such as SIGQUIT. Repeat */ + } + return ret; +} + static int FAST_FUNC builtin_wait(char **argv) { - int ret = EXIT_SUCCESS; + int ret; int status; argv = skip_dash_dash(argv); @@ -9431,74 +9520,41 @@ static int FAST_FUNC builtin_wait(char **argv) * ^C <-- after ~4 sec from keyboard * $ */ - while (1) { - int sig; - sigset_t oldset, allsigs; - - /* waitpid is not interruptible by SA_RESTARTed - * signals which we use. Thus, this ugly dance: - */ - - /* Make sure possible SIGCHLD is stored in kernel's - * pending signal mask before we call waitpid. - * Or else we may race with SIGCHLD, lose it, - * and get stuck in sigwaitinfo... - */ - sigfillset(&allsigs); - sigprocmask(SIG_SETMASK, &allsigs, &oldset); - - if (!sigisemptyset(&G.pending_set)) { - /* Crap! we raced with some signal! */ - // sig = 0; - goto restore; - } - - checkjobs(NULL); /* waitpid(WNOHANG) inside */ - if (errno == ECHILD) { - sigprocmask(SIG_SETMASK, &oldset, NULL); - break; - } - - /* Wait for SIGCHLD or any other signal */ - //sig = sigwaitinfo(&allsigs, NULL); - /* It is vitally important for sigsuspend that SIGCHLD has non-DFL handler! */ - /* Note: sigsuspend invokes signal handler */ - sigsuspend(&oldset); - restore: - sigprocmask(SIG_SETMASK, &oldset, NULL); - - /* So, did we get a signal? */ - //if (sig > 0) - // raise(sig); /* run handler */ - sig = check_and_run_traps(); - if (sig /*&& sig != SIGCHLD - always true */) { - /* see note 2 */ - ret = 128 + sig; - break; - } - /* SIGCHLD, or no signal, or ignored one, such as SIGQUIT. Repeat */ - } - return ret; + return wait_for_child_or_signal(0 /*(no pid to wait for)*/); } - /* This is probably buggy wrt interruptible-ness */ - while (*argv) { + /* TODO: support "wait %jobspec" */ + do { pid_t pid = bb_strtou(*argv, NULL, 10); - if (errno) { + if (errno || pid <= 0) { /* mimic bash message */ bb_error_msg("wait: '%s': not a pid or valid job spec", *argv); return EXIT_FAILURE; } - if (waitpid(pid, &status, 0) == pid) { + /* Do we have such child? */ + ret = waitpid(pid, &status, WNOHANG); + if (ret < 0) { + /* No */ + if (errno == ECHILD) { + /* Example: "wait 1". mimic bash message */ + bb_error_msg("wait: pid %d is not a child of this shell", (int)pid); + } else { + /* ??? */ + bb_perror_msg("wait %s", *argv); + } + ret = 127; + } else if (ret == 0) { + /* Yes, and it still runs */ + ret = wait_for_child_or_signal(pid); + } else { + /* Yes, and it just exited */ + process_wait_result(NULL, pid, status); ret = WEXITSTATUS(status); if (WIFSIGNALED(status)) ret = 128 + WTERMSIG(status); - } else { - bb_perror_msg("wait %s", *argv); - ret = 127; } argv++; - } + } while (*argv); return ret; } diff --git a/shell/hush_test/hush-misc/wait1.right b/shell/hush_test/hush-misc/wait1.right new file mode 100644 index 000000000..20f514da0 --- /dev/null +++ b/shell/hush_test/hush-misc/wait1.right @@ -0,0 +1,2 @@ +0 +[1] Running sleep 2 diff --git a/shell/hush_test/hush-misc/wait1.tests b/shell/hush_test/hush-misc/wait1.tests new file mode 100755 index 000000000..f9cf6d48c --- /dev/null +++ b/shell/hush_test/hush-misc/wait1.tests @@ -0,0 +1,3 @@ +sleep 2 & sleep 1 & wait $! +echo $? +jobs diff --git a/shell/hush_test/hush-misc/wait2.right b/shell/hush_test/hush-misc/wait2.right new file mode 100644 index 000000000..f018c2c98 --- /dev/null +++ b/shell/hush_test/hush-misc/wait2.right @@ -0,0 +1,2 @@ +0 +[1] Running sleep 3 diff --git a/shell/hush_test/hush-misc/wait2.tests b/shell/hush_test/hush-misc/wait2.tests new file mode 100755 index 000000000..be20f95a5 --- /dev/null +++ b/shell/hush_test/hush-misc/wait2.tests @@ -0,0 +1,4 @@ +sleep 3 & sleep 2 & sleep 1 +wait $! +echo $? +jobs diff --git a/shell/hush_test/hush-misc/wait3.right b/shell/hush_test/hush-misc/wait3.right new file mode 100644 index 000000000..5437b1d73 --- /dev/null +++ b/shell/hush_test/hush-misc/wait3.right @@ -0,0 +1,2 @@ +3 +[1] Running sleep 2 diff --git a/shell/hush_test/hush-misc/wait3.tests b/shell/hush_test/hush-misc/wait3.tests new file mode 100755 index 000000000..ac541c3fc --- /dev/null +++ b/shell/hush_test/hush-misc/wait3.tests @@ -0,0 +1,3 @@ +sleep 2 & (sleep 1;exit 3) & wait $! +echo $? +jobs -- cgit v1.2.3-55-g6feb From 9db74e49e5b462089c6eec0182d819c0d4708e57 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 28 Oct 2016 22:39:12 +0200 Subject: hush: fix "(sleep 1; exit 3) & sleep 2; echo $?; wait $!; echo $?" "wait $!" may be just a bit too late: backgrounded $! is gone. Do not bomb out in this case. Signed-off-by: Denys Vlasenko --- shell/hush.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/shell/hush.c b/shell/hush.c index 1f8a3e607..7c2f157b8 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -9529,13 +9529,22 @@ static int FAST_FUNC builtin_wait(char **argv) if (errno || pid <= 0) { /* mimic bash message */ bb_error_msg("wait: '%s': not a pid or valid job spec", *argv); - return EXIT_FAILURE; + ret = EXIT_FAILURE; + continue; /* bash checks all argv[] */ } /* Do we have such child? */ ret = waitpid(pid, &status, WNOHANG); if (ret < 0) { /* No */ if (errno == ECHILD) { + if (G.last_bg_pid > 0 && pid == G.last_bg_pid) { + /* "wait $!" but last bg task has already exited. Try: + * (sleep 1; exit 3) & sleep 2; echo $?; wait $!; echo $? + * In bash it prints exitcode 0, then 3. + */ + ret = 0; /* FIXME */ + continue; + } /* Example: "wait 1". mimic bash message */ bb_error_msg("wait: pid %d is not a child of this shell", (int)pid); } else { @@ -9543,7 +9552,9 @@ static int FAST_FUNC builtin_wait(char **argv) bb_perror_msg("wait %s", *argv); } ret = 127; - } else if (ret == 0) { + continue; /* bash checks all argv[] */ + } + if (ret == 0) { /* Yes, and it still runs */ ret = wait_for_child_or_signal(pid); } else { @@ -9553,8 +9564,7 @@ static int FAST_FUNC builtin_wait(char **argv) if (WIFSIGNALED(status)) ret = 128 + WTERMSIG(status); } - argv++; - } while (*argv); + } while (*++argv); return ret; } -- cgit v1.2.3-55-g6feb From 493b9cae808093ff5d89b5fb794e3991859e7df6 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 30 Oct 2016 18:27:14 +0100 Subject: ash: make popfile() anfter popallfiles() safe In this example: ash -c 'readonly x; echo $(command eval x=2)' evalstring() is called after forkchild(), which calls popallfiles(). On exception, evalstring() will popfile(). Signed-off-by: Denys Vlasenko --- shell/ash.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index fc1b5d927..0c8480587 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -10125,6 +10125,9 @@ popfile(void) { struct parsefile *pf = g_parsefile; + if (pf == &basepf) + return; + INT_OFF; if (pf->pf_fd >= 0) close(pf->pf_fd); @@ -12286,7 +12289,7 @@ expandstr(const char *ps) static int evalstring(char *s, int flags) { - struct jmploc *volatile savehandler = exception_handler; + struct jmploc *volatile savehandler; struct jmploc jmploc; int ex; @@ -12307,10 +12310,10 @@ evalstring(char *s, int flags) * But if we skip popfile(), we hit EOF in eval's string, and exit. */ savehandler = exception_handler; - exception_handler = &jmploc; ex = setjmp(jmploc.loc); if (ex) goto out; + exception_handler = &jmploc; while ((n = parsecmd(0)) != NODE_EOF) { int i; -- cgit v1.2.3-55-g6feb From 474ed06c3939391cbfd7b70bf4960403ae166762 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 30 Oct 2016 18:30:29 +0100 Subject: ash: fix bit-rotten debug infrastructure DEBUG = 2 output was a bit messed up Signed-off-by: Denys Vlasenko --- shell/ash.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index 0c8480587..d617168b9 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -844,13 +844,8 @@ trace_vprintf(const char *fmt, va_list va) { if (debug != 1) return; - if (DEBUG_TIME) - fprintf(tracefile, "%u ", (int) time(NULL)); - if (DEBUG_PID) - fprintf(tracefile, "[%u] ", (int) getpid()); - if (DEBUG_SIG) - fprintf(tracefile, "pending s:%d i:%d(supp:%d) ", pending_sig, pending_int, suppress_int); vfprintf(tracefile, fmt, va); + fprintf(tracefile, "\n"); } static void @@ -1243,11 +1238,10 @@ ash_vmsg_and_raise(int cond, const char *msg, va_list ap) { #if DEBUG if (msg) { - TRACE(("ash_vmsg_and_raise(%d, \"", cond)); + TRACE(("ash_vmsg_and_raise(%d):", cond)); TRACEV((msg, ap)); - TRACE(("\") pid=%d\n", getpid())); } else - TRACE(("ash_vmsg_and_raise(%d, NULL) pid=%d\n", cond, getpid())); + TRACE(("ash_vmsg_and_raise(%d):NULL\n", cond)); if (msg) #endif ash_vmsg(msg, ap); @@ -13422,16 +13416,15 @@ int ash_main(int argc UNUSED_PARAM, char **argv) goto state4; } exception_handler = &jmploc; -#if DEBUG - opentrace(); - TRACE(("Shell args: ")); - trace_puts_args(argv); -#endif rootpid = getpid(); init(); setstackmark(&smark); procargs(argv); +#if DEBUG + TRACE(("Shell args: ")); + trace_puts_args(argv); +#endif if (argv[0] && argv[0][0] == '-') isloginsh = 1; -- cgit v1.2.3-55-g6feb From d4f3db9427c443b2709fc9a00bc46d8a71be806b Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 30 Oct 2016 18:41:01 +0100 Subject: ash: if using libc glob(), skip it if no metachars are in word This saves making tons of pointless stat() calls function old new delta expandarg 888 921 +33 Signed-off-by: Denys Vlasenko --- shell/ash.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/shell/ash.c b/shell/ash.c index d617168b9..ecd2146e4 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -7047,6 +7047,21 @@ expandmeta(struct strlist *str /*, int flag*/) if (fflag) goto nometa; + + /* Avoid glob() (and thus, stat() et al) for words like "echo" */ + p = str->text; + while (*p) { + if (*p == '*') + goto need_glob; + if (*p == '?') + goto need_glob; + if (*p == '[') + goto need_glob; + p++; + } + goto nometa; + + need_glob: INT_OFF; p = preglob(str->text, RMESCAPE_ALLOC | RMESCAPE_HEAP); // GLOB_NOMAGIC (GNU): if no *?[ chars in pattern, return it even if no match -- cgit v1.2.3-55-g6feb From a92a74961d838209f3468d10426bc945ba26070c Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 30 Oct 2016 22:31:30 +0100 Subject: man: allow nroff and tbl commands be overridden; unmangle writing to files Parse this in config files: DEFINE col ... DEFINE tbl ... DEFINE nroff ... Add width options to nroff command line. Use "tbl", not "gtbl", as default tbl command. Export GROFF_NO_SGR=1 and use "col -b -p -x" instead of pager when writing to file. function old new delta man_main 735 863 +128 if_redefined - 64 +64 show_manpage 199 169 -30 ------------------------------------------------------------------------------ (add/remove: 1/0 grow/shrink: 1/1 up/down: 192/-30) Total: 162 bytes Signed-off-by: Denys Vlasenko --- miscutils/man.c | 104 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 79 insertions(+), 25 deletions(-) diff --git a/miscutils/man.c b/miscutils/man.c index f705dd31e..01382c4d7 100644 --- a/miscutils/man.c +++ b/miscutils/man.c @@ -11,6 +11,7 @@ //usage: "\n -w Show page locations" #include "libbb.h" +#include "common_bufsiz.h" enum { OPT_a = 1, /* all */ @@ -18,7 +19,6 @@ enum { }; /* This is what I see on my desktop system being executed: - ( echo ".ll 12.4i" echo ".nr LL 12.4i" @@ -28,11 +28,39 @@ echo ".\\\"" echo ".pl \n(nlu+10" ) | gtbl | nroff -Tlatin1 -mandoc | less -*/ +On another system I see this: -static int show_manpage(const char *pager, char *man_filename, int man, int level); +... | tbl | nroff -mandoc -rLL=n -rLT=n -Tutf8 | less -static int run_pipe(const char *pager, char *man_filename, int man, int level) +where is screen width minus 5. +Replacing "DEFINE nroff nroff -mandoc" in /etc/man_db.conf +changes "nroff -mandoc" part; -rLL=n, -rLT=n and -Tutf8 parts are +appended to the user-specified command. + +Redirecting to a pipe or file sets GROFF_NO_SGR=1 to prevent color escapes, +and uses "col -b -p -x" instead of pager, this filters out backspace +and underscore tricks. +*/ + +struct globals { + const char *col; + const char *tbl; + const char *nroff; + const char *pager; +} FIX_ALIASING; +#define G (*(struct globals*)bb_common_bufsiz1) +#define INIT_G() do { \ + setup_common_bufsiz(); \ + G.col = "col"; \ + G.tbl = "tbl"; \ + /* replaced -Tlatin1 with -Tascii for non-UTF8 displays */; \ + G.nroff = "nroff -mandoc -Tascii"; \ + G.pager = ENABLE_LESS ? "less" : "more"; \ +} while (0) + +static int show_manpage(char *man_filename, int man, int level); + +static int run_pipe(char *man_filename, int man, int level) { char *cmd; @@ -95,7 +123,7 @@ static int run_pipe(const char *pager, char *man_filename, int man, int level) man_filename = xasprintf("%s/%s", man_filename, linkname); free(line); /* Note: we leak "new" man_filename string as well... */ - if (show_manpage(pager, man_filename, man, level + 1)) + if (show_manpage(man_filename, man, level + 1)) return 1; /* else: show the link, it's better than nothing */ } @@ -103,20 +131,29 @@ static int run_pipe(const char *pager, char *man_filename, int man, int level) ordinary_manpage: close(STDIN_FILENO); open_zipped(man_filename, /*fail_if_not_compressed:*/ 0); /* guaranteed to use fd 0 (STDIN_FILENO) */ - /* "2>&1" is added so that nroff errors are shown in pager too. - * Otherwise it may show just empty screen */ - cmd = xasprintf( - /* replaced -Tlatin1 with -Tascii for non-UTF8 displays */ - man ? "gtbl | nroff -Tascii -mandoc 2>&1 | %s" - : "%s", - pager); + if (man) { + /* "man man" formats to screen width. + * "man man >file" formats to default 80 columns. + * "man man | cat" formats to default 80 columns. + */ + int w = get_terminal_width(STDOUT_FILENO); + if (w > 10) + w -= 2; + /* "2>&1" is added so that nroff errors are shown in pager too. + * Otherwise it may show just empty screen */ + cmd = xasprintf("%s | %s -rLL=%un -rLT=%un 2>&1 | %s", + G.tbl, G.nroff, w, w, + G.pager); + } else { + cmd = xstrdup(G.pager); + } system(cmd); free(cmd); return 1; } /* man_filename is of the form "/dir/dir/dir/name.s" */ -static int show_manpage(const char *pager, char *man_filename, int man, int level) +static int show_manpage(char *man_filename, int man, int level) { #if SEAMLESS_COMPRESSION /* We leak this allocation... */ @@ -125,26 +162,26 @@ static int show_manpage(const char *pager, char *man_filename, int man, int leve #endif #if ENABLE_FEATURE_SEAMLESS_LZMA - if (run_pipe(pager, filename_with_zext, man, level)) + if (run_pipe(filename_with_zext, man, level)) return 1; #endif #if ENABLE_FEATURE_SEAMLESS_XZ strcpy(ext, "xz"); - if (run_pipe(pager, filename_with_zext, man, level)) + if (run_pipe(filename_with_zext, man, level)) return 1; #endif #if ENABLE_FEATURE_SEAMLESS_BZ2 strcpy(ext, "bz2"); - if (run_pipe(pager, filename_with_zext, man, level)) + if (run_pipe(filename_with_zext, man, level)) return 1; #endif #if ENABLE_FEATURE_SEAMLESS_GZ strcpy(ext, "gz"); - if (run_pipe(pager, filename_with_zext, man, level)) + if (run_pipe(filename_with_zext, man, level)) return 1; #endif - return run_pipe(pager, man_filename, man, level); + return run_pipe(man_filename, man, level); } static char **add_MANPATH(char **man_path_list, int *count_mp, char *path) @@ -182,11 +219,20 @@ static char **add_MANPATH(char **man_path_list, int *count_mp, char *path) return man_path_list; } +static const char *if_redefined(const char *var, const char *key, const char *line) +{ + if (!is_prefixed_with(line, key)) + return var; + line += strlen(key); + if (!isspace(line[0])) + return var; + return xstrdup(skip_whitespace(line)); +} + int man_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int man_main(int argc UNUSED_PARAM, char **argv) { parser_t *parser; - const char *pager = ENABLE_LESS ? "less" : "more"; char *sec_list; char *cur_path, *cur_sect; char **man_path_list; @@ -195,6 +241,8 @@ int man_main(int argc UNUSED_PARAM, char **argv) int opt, not_found; char *token[2]; + INIT_G(); + opt_complementary = "-1"; /* at least one argument */ opt = getopt32(argv, "+aw"); argv += optind; @@ -228,9 +276,10 @@ int man_main(int argc UNUSED_PARAM, char **argv) if (!token[1]) continue; if (strcmp("DEFINE", token[0]) == 0) { - if (is_prefixed_with(token[1], "pager")) { - pager = xstrdup(skip_whitespace(token[1] + 5)); - } + G.col = if_redefined(G.tbl , "col", token[1]); + G.tbl = if_redefined(G.tbl , "tbl", token[1]); + G.nroff = if_redefined(G.nroff, "nroff", token[1]); + G.pager = if_redefined(G.pager, "pager", token[1]); } else if (strcmp("MANDATORY_MANPATH"+10, token[0]) == 0 /* "MANPATH"? */ || strcmp("MANDATORY_MANPATH", token[0]) == 0 @@ -250,7 +299,12 @@ int man_main(int argc UNUSED_PARAM, char **argv) if (!env_pager) env_pager = getenv("PAGER"); if (env_pager) - pager = env_pager; + G.pager = env_pager; + } + + if (!isatty(STDOUT_FILENO)) { + putenv((char*)"GROFF_NO_SGR=1"); + G.pager = xasprintf("%s -b -p -x", G.col); } not_found = 0; @@ -259,7 +313,7 @@ int man_main(int argc UNUSED_PARAM, char **argv) cur_mp = 0; if (strchr(*argv, '/')) { - found = show_manpage(pager, *argv, /*man:*/ 1, 0); + found = show_manpage(*argv, /*man:*/ 1, 0); goto check_found; } while ((cur_path = man_path_list[cur_mp++]) != NULL) { @@ -280,7 +334,7 @@ int man_main(int argc UNUSED_PARAM, char **argv) sect_len, cur_sect, *argv, sect_len, cur_sect); - found_here = show_manpage(pager, man_filename, cat0man1, 0); + found_here = show_manpage(man_filename, cat0man1, 0); found |= found_here; cat0man1 += found_here + 1; free(man_filename); -- cgit v1.2.3-55-g6feb From 7c3c92c533b65d4c29f2990915c9c424c3f6629d Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Oct 2016 01:52:18 +0100 Subject: man: make width selection more thorough; explain how to override it Fedora's "man CMD >file" still uses terminal width, not 80 (but disables formatting), this change mimics that. Signed-off-by: Denys Vlasenko --- libbb/xfuncs.c | 47 ++++++++++++++++++++++++++++++++++++++--------- miscutils/man.c | 13 ++++++------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/libbb/xfuncs.c b/libbb/xfuncs.c index 3f9a84ad4..45650edba 100644 --- a/libbb/xfuncs.c +++ b/libbb/xfuncs.c @@ -237,16 +237,27 @@ ssize_t FAST_FUNC full_write2_str(const char *str) static int wh_helper(int value, int def_val, const char *env_name, int *err) { - if (value == 0) { - char *s = getenv(env_name); - if (s) { - value = atoi(s); - /* If LINES/COLUMNS are set, pretend that there is - * no error getting w/h, this prevents some ugly - * cursor tricks by our callers */ - *err = 0; - } + /* Envvars override even if "value" from ioctl is valid (>0). + * Rationale: it's impossible to guess what user wants. + * For example: "man CMD | ...": should "man" format output + * to stdout's width? stdin's width? /dev/tty's width? 80 chars? + * We _cant_ know it. If "..." saves text for e.g. email, + * then it's probably 80 chars. + * If "..." is, say, "grep -v DISCARD | $PAGER", then user + * would prefer his tty's width to be used! + * + * Since we don't know, at least allow user to do this: + * "COLUMNS=80 man CMD | ..." + */ + char *s = getenv(env_name); + if (s) { + value = atoi(s); + /* If LINES/COLUMNS are set, pretend that there is + * no error getting w/h, this prevents some ugly + * cursor tricks by our callers */ + *err = 0; } + if (value <= 1 || value >= 30000) value = def_val; return value; @@ -258,6 +269,20 @@ int FAST_FUNC get_terminal_width_height(int fd, unsigned *width, unsigned *heigh { struct winsize win; int err; + int close_me = -1; + + if (fd == -1) { + if (isatty(STDOUT_FILENO)) + fd = STDOUT_FILENO; + else + if (isatty(STDERR_FILENO)) + fd = STDERR_FILENO; + else + if (isatty(STDIN_FILENO)) + fd = STDIN_FILENO; + else + close_me = fd = open("/dev/tty", O_RDONLY); + } win.ws_row = 0; win.ws_col = 0; @@ -268,6 +293,10 @@ int FAST_FUNC get_terminal_width_height(int fd, unsigned *width, unsigned *heigh *height = wh_helper(win.ws_row, 24, "LINES", &err); if (width) *width = wh_helper(win.ws_col, 80, "COLUMNS", &err); + + if (close_me >= 0) + close(close_me); + return err; } int FAST_FUNC get_terminal_width(int fd) diff --git a/miscutils/man.c b/miscutils/man.c index 01382c4d7..adb7770b4 100644 --- a/miscutils/man.c +++ b/miscutils/man.c @@ -9,6 +9,8 @@ //usage: "Format and display manual page\n" //usage: "\n -a Display all pages" //usage: "\n -w Show page locations" +//usage: "\n" +//usage: "\n$COLUMNS overrides output width" #include "libbb.h" #include "common_bufsiz.h" @@ -53,7 +55,7 @@ struct globals { setup_common_bufsiz(); \ G.col = "col"; \ G.tbl = "tbl"; \ - /* replaced -Tlatin1 with -Tascii for non-UTF8 displays */; \ + /* replaced -Tlatin1 with -Tascii for non-UTF8 displays */ \ G.nroff = "nroff -mandoc -Tascii"; \ G.pager = ENABLE_LESS ? "less" : "more"; \ } while (0) @@ -132,15 +134,12 @@ static int run_pipe(char *man_filename, int man, int level) close(STDIN_FILENO); open_zipped(man_filename, /*fail_if_not_compressed:*/ 0); /* guaranteed to use fd 0 (STDIN_FILENO) */ if (man) { - /* "man man" formats to screen width. - * "man man >file" formats to default 80 columns. - * "man man | cat" formats to default 80 columns. - */ - int w = get_terminal_width(STDOUT_FILENO); + int w = get_terminal_width(-1); if (w > 10) w -= 2; /* "2>&1" is added so that nroff errors are shown in pager too. - * Otherwise it may show just empty screen */ + * Otherwise it may show just empty screen. + */ cmd = xasprintf("%s | %s -rLL=%un -rLT=%un 2>&1 | %s", G.tbl, G.nroff, w, w, G.pager); -- cgit v1.2.3-55-g6feb From 2e6af549715f5d7b4c2ab204e46c8b8f6f057045 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 31 Oct 2016 14:05:34 +0100 Subject: man: remove -Tascii from nroff invocation Signed-off-by: Denys Vlasenko --- miscutils/man.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/miscutils/man.c b/miscutils/man.c index adb7770b4..932f0b794 100644 --- a/miscutils/man.c +++ b/miscutils/man.c @@ -30,6 +30,8 @@ echo ".\\\"" echo ".pl \n(nlu+10" ) | gtbl | nroff -Tlatin1 -mandoc | less +Some systems use -Tascii. + On another system I see this: ... | tbl | nroff -mandoc -rLL=n -rLT=n -Tutf8 | less @@ -55,8 +57,8 @@ struct globals { setup_common_bufsiz(); \ G.col = "col"; \ G.tbl = "tbl"; \ - /* replaced -Tlatin1 with -Tascii for non-UTF8 displays */ \ - G.nroff = "nroff -mandoc -Tascii"; \ + /* Removed -Tlatin1. Assuming system nroff has suitable default */ \ + G.nroff = "nroff -mandoc"; \ G.pager = ENABLE_LESS ? "less" : "more"; \ } while (0) -- cgit v1.2.3-55-g6feb