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

Fix incorrect comment on abort_internal #86760

Closed
wants to merge 1 commit into from

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jun 30, 2021

This was wrong in several ways:

  1. abort() still calls signal handlers
  2. abort() no longer flushes streams (since glibc 2.27)

Rather than trying to keep it up-to-date with the man page, it now just says
"behaves the same as libc's abort".

This would have saved me quite a lot of time earlier today debugging.

cc #81895, #40230

This was wrong in several ways:
1. abort() still calls signal handlers
2. abort() no longer flushes streams (since glibc 2.27)

Rather than trying to keep it up-to-date with the man page, it now just says
"behaves the same as libc's abort".
@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 30, 2021
@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jun 30, 2021

r? @m-ou-se

@rust-highfive rust-highfive assigned m-ou-se and unassigned dtolnay Jun 30, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jul 5, 2021

#85377 also modifies this documentation. Do those changes suffice?

@m-ou-se m-ou-se added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 5, 2021
@jyn514
Copy link
Member Author

jyn514 commented Jul 5, 2021

Hmm, well, it seems unfortunate that it keeps the whole history of libc's abort in the docs (which means they can get out of date), but otherwise the comment is a lot more helpful than before.

@jyn514 jyn514 closed this Jul 5, 2021
@jyn514 jyn514 deleted the sigabrt branch July 5, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants