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

cleanup: remove exit.h, use unistd.h instead. #44

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

alanpost
Copy link
Contributor

@alanpost alanpost commented Jul 23, 2019

This PR removes exit.h to replace it with unistd.h. The same approach was used in PR #43, which removes readwrite.h, so this PR builds on that one. They touch a lot of the same files.

exit.h only includes system-defined functions, and since _exit is so basic to a process it shows up all over the codebase. It's a reasonable start for removing redeclaration of system functions (issue #7), as it touches a lot of files and includes a system header that has the bulk of the declarations we no longer want to shadow.

Alas, exit.h removal cannot be done before PR #43, as including unistd.h causes compiler errors when including the readwrite.h header. As a result I'm experimenting here with separate PRs for otherwise incidentally staged commits.

@alanpost alanpost added this to the 1.07 milestone Jul 23, 2019
@alanpost alanpost modified the milestones: 1.07, 1.08 Jul 23, 2019
@alanpost alanpost added the build label Jul 23, 2019
@alanpost alanpost changed the title cleanup: remove exit.h, use in unistd.h. cleanup: remove exit.h, use unistd.h instead. Jul 23, 2019
@DerDakon
Copy link
Member

The target branch of this PR is not master, which looks odd.

@alanpost alanpost force-pushed the pr-remove-exit-h branch 2 times, most recently from c42db5a to 50c9b59 Compare August 4, 2019 22:09
@alanpost alanpost force-pushed the pr-remove-readwrite-h branch 2 times, most recently from b9e235a to c3374c3 Compare August 6, 2019 13:58
@alanpost alanpost force-pushed the pr-remove-readwrite-h branch 2 times, most recently from c7d7ebf to a1e7caa Compare August 24, 2019 14:37
@alanpost alanpost changed the base branch from pr-remove-readwrite-h to master August 24, 2019 15:30
@alanpost alanpost modified the milestones: 1.08, 1.9 Aug 24, 2019
@alanpost
Copy link
Contributor Author

The target branch of this PR is not master, which looks odd.

As this series of commits is split in to those suitable for 1.08, and those targeted for 1.9, I've set the target branch to master. I have cherry-picked the 1.08 readwrite.h commit from PR # 80 instead of the 1.9 readwrite.h PR #43 , as this exit.h PR absolutely requires something be done about the declarations in readwrite.h but doesn't require removing that header in favor of unistd.h.

As a result each PR (#43 and #44) now have fewer dependencies on the other. I've updated the milestone for this PR to the 1.9 release. I've submitted PR #79 for the beginning of this series, with the minimal change for the 1.08 release.

@alanpost alanpost force-pushed the pr-remove-exit-h branch 3 times, most recently from f5da7d1 to d884a63 Compare August 25, 2019 14:14
_exit(2) is declared in unistd.h.  Instead of redeclaring it in
exit.h, include that system header to get the correct function
signature.
Both of these programs call _exit but don't include the exit.h header
where it is declared.

Inspired by Gentoo's 1.06-exit.patch.
Neither of these programs calls _exit.  forward calls
strerr_die2x with a status of 0, in order to send a
log message.  maildirwatch runs in a loop and never
exits.
_exit(2) is declared in unistd.h.
Copy link

@josuah josuah left a comment

Choose a reason for hiding this comment

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

It looks like DJB was putting high pressure on avoiding libc and only rely on its own embedded libc. I guess that nowaday, we can safely rely on <unistd.h> having _exit().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants