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 readwrite.h, use unistd.h instead. #43

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alanpost
Copy link
Contributor

The header readwrite.h (re)declares read(2) and write(2). We should use the system-provided header for these declarations, unistd.h.

This patch systematically removes readwrite.h and includes unistd.h instead.

@DerDakon
Copy link
Member

Both share a common problem: they touch a lot of files, which easily will break external patches that add their own headers. I would suggest that we split this into 2 PRs, and #44 as well: one that modifies readwrite.h to just #include <unistd.h>, and one that entirely get's rid of that header. This way we can merge the first one e.g. in 1.08, adding correctness while still allowing external patches to easily be applied. And the other one could come in a later release when we decided to allow patch breakage.

@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 modified the milestones: 1.08, 1.9 Aug 24, 2019
@alanpost
Copy link
Contributor Author

I would suggest that we split this into 2 PRs, and #44 as well: one that modifies readwrite.h to just #include <unistd.h>, and one that entirely get's rid of that header.

I've done this here, and as a consequence set this PR to the 1.9 release. I've submitted PR #80 for the beginning of this series, with the minimal change as you suggest, for the 1.08 release.

read(2) and write(2) are declared in unistd.h.
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.

None yet

2 participants