Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

man.sh: revert trapping SIGPIPE #1283

Merged
merged 1 commit into from
Jun 28, 2024
Merged

man.sh: revert trapping SIGPIPE #1283

merged 1 commit into from
Jun 28, 2024

Conversation

concussious
Copy link
Contributor

You teach me a lot with fun puzzles. That really adds a lot to my quality of life. Thanks yet again @emaste!

@concussious
Copy link
Contributor Author

Cc @wosch @jillest

@emaste
Copy link
Member

emaste commented Jun 10, 2024

Confirm this addresses the issue I reported in PR279542

@concussious
Copy link
Contributor Author

concussious commented Jun 10, 2024

There are other ways to solve this problem (you could trap 'exit 0' SIGPIPE), but I don't understand the use case for them.

@emaste
Copy link
Member

emaste commented Jun 11, 2024

Yeah, I'm not sure either -- I'll defer to @wosch for comment.

@concussious
Copy link
Contributor Author

Confirm that this doesn't reintroduce core dumping on % man 7 salt, which is half a million lines(!), (py311-salt on 15-current) as described in PR223516.

Copy link
Member

@jillest jillest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the main purpose of the default action of SIGPIPE appears to be to terminate processes writing output that is no longer useful, without generating error messages.

Together with set -o pipefail this can still cause non-zero exit statuses, but I don't think this is an issue in this script.

@jillest
Copy link
Member

jillest commented Jun 12, 2024

There are other ways to solve this problem (you could trap 'exit 0' SIGPIPE), but I don't understand the use case for them.

The script itself does not commonly write to a closed pipe and non-empty trap actions are turned into the default action in child processes, so trap 'exit 0' PIPE has almost the same effect as not doing anything with SIGPIPE. In either case, if SIGPIPE was inherited as ignored, it will stay that way. So I like it better to do nothing with SIGPIPE.

@concussious
Copy link
Contributor Author

Thanks @jillest!

@bsdimp bsdimp added the ready label Jun 17, 2024
@bsdimp bsdimp self-assigned this Jun 17, 2024
PR:		279542
Fixes:		14a5c10
Reported by:	emaste
Reviewed by: imp, emaste, jilles
Pull Request: freebsd#1283
@freebsd-git freebsd-git merged commit a85d870 into freebsd:main Jun 28, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants