Bug 223516 - man(1) ignores the exit status of groff
Summary: man(1) ignores the exit status of groff
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 11.1-STABLE
Hardware: Any Any
: --- Affects Only Me
Assignee: Wolfram Schneider
URL: https://reviews.freebsd.org/D44798
Keywords:
Depends on: 223515 224270
Blocks: 278378
  Show dependency treegraph
 
Reported: 2017-11-08 10:18 UTC by Wolfram Schneider
Modified: 2024-06-11 16:52 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Wolfram Schneider freebsd_committer freebsd_triage 2017-11-08 10:18:58 UTC
I tried to read the manpage of salt(7). troff crashes with signal 6, leaves a core file. But the man(1) command report an exit status 0. It shouldn’t, better use status 2 or something similar.

$ man -M ~/man salt  >/dev/null; echo $?
grotty:<standard input> (<standard input>):1266315: character above first line discarded
troff: Failed assertion at line 510, file `/usr/src/gnu/usr.bin/groff/src/roff/troff/../../../../../../contrib/groff/src/roff/troff/input.cpp'.
grotty:<standard input> (<standard input>):1994486:warning: no final `x stop' command
groff: troff: Signal 6 (core dumped)
0

groff(1) correctly reports an error and an exit status 2:

$ zcat ~/man/man7/salt.7.gz | groff >/dev/null 2>&1; echo $?
2
Comment 1 Wolfram Schneider freebsd_committer freebsd_triage 2017-12-08 09:18:34 UTC
I think the problem is in the function

line 456:
   456                  eval "$cattool $manpage | $pipeline"
   457                  ret=$?

we are checking only the status of the last command, which is the $PAGER (e.g. less(1))

eval '/usr/bin/zcat /home/wosch/man/man7/salt.7.gz | tbl | groff -S -P-h -Wall -mtty-char -man  -Tascii -P-c | less'

and ignore the exit status of groff (and tbl, zcat - any command in the pipeline)

The bash has an option to fail on pipe errors

$ set -o pipefail

unfortunately, our /bin/sh implementation does not have this feature.


There are workaround for shell which don’t check the pipe status, as described in
https://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another

(((((exec 3>&- 4>&-; someprog); echo $? >&3) | filter >&4) 3>&1) | (read xs; exit $xs)) 4>&1

but this looks bizarre and you have to do it for every command in the pipe line.
Comment 2 Jilles Tjoelker freebsd_committer freebsd_triage 2019-03-26 22:43:35 UTC
sh now has pipefail, but you do need the additional logic from my commit message to ignore SIGPIPE exits because pagers may exit before reading all their input. This additional logic is definitely simpler than the logic to pass through exit status without pipefail, though.
Comment 3 commit-hook freebsd_committer freebsd_triage 2024-04-20 08:30:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=14a5c1068d3751173dc41f3097b12e95791b2160

commit 14a5c1068d3751173dc41f3097b12e95791b2160
Author:     Wolfram Schneider <wosch@FreeBSD.org>
AuthorDate: 2024-04-20 08:24:58 +0000
Commit:     Wolfram Schneider <wosch@FreeBSD.org>
CommitDate: 2024-04-20 08:30:33 +0000

    man: do not ignore the exit status of roff tools

    PR:             223516
    Approved by:    emaste, bapt
    Differential Revision:  https://reviews.freebsd.org/D44798

 usr.bin/man/man.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2024-06-03 16:35:32 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=19f8e6b9a33761bddec9c8034d2ae9b1cd80f5cb

commit 19f8e6b9a33761bddec9c8034d2ae9b1cd80f5cb
Author:     Wolfram Schneider <wosch@FreeBSD.org>
AuthorDate: 2024-04-20 08:24:58 +0000
Commit:     Wolfram Schneider <wosch@FreeBSD.org>
CommitDate: 2024-06-03 16:22:52 +0000

    man: do not ignore the exit status of roff tools

    PR:             223516
    Approved by:    emaste, bapt
    Differential Revision:  https://reviews.freebsd.org/D44798

    (cherry picked from commit 14a5c1068d3751173dc41f3097b12e95791b2160)

 usr.bin/man/man.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)