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

Fixed ptrace::Request cfg conditions #892

Merged
merged 1 commit into from
Apr 29, 2018

Conversation

dalance
Copy link
Contributor

@dalance dalance commented Apr 26, 2018

The cfg condition of ptrace::Request seems to be different from libc.
For example, PTRACE_GETREGS is defined by libc to i686-unknown-linux-gnu target, but it is not defined in ptrace::Request.
I tried to change the cfg condition to the same as libc's condition.
I thinks this change covers the definitions under src/unix/notbsd directory in libc repository.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Could you please line-wrap everything to 80 columns? That will make this PR easier to review. Also, it needs a CHANGELOG entry.

@dalance
Copy link
Contributor Author

dalance commented Apr 29, 2018

@asomers Thnak you for your comment, I applied line-wrap, and added CHANGELOG entry.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

The changes look good. Just tweak the wording of the changelog and squash your commits, and I'll merge them.

CHANGELOG.md Outdated
@@ -51,6 +51,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#833](https://github.com/nix-rust/nix/pull/833))
- `ioctl_write_int!` now properly supports passing a `c_ulong` as the parameter on Linux non-musl targets
([#833](https://github.com/nix-rust/nix/pull/833))
- Fixed `ptrace::Request` cfg conditions
Copy link
Member

Choose a reason for hiding this comment

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

How about "Enabled more ptrace::Request definitions for uncommon Linux platforms"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change to it.
Should it be moved to Added or Changed section?

Copy link
Member

Choose a reason for hiding this comment

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

Ehh, it could go either way. Maybe Changed would be best.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it to Changed setcion.

@dalance dalance force-pushed the fix_ptrace_request_cfg branch from 8e0e904 to 42ffc88 Compare April 29, 2018 03:28
@dalance dalance force-pushed the fix_ptrace_request_cfg branch from 42ffc88 to 1dda866 Compare April 29, 2018 03:32
@dalance
Copy link
Contributor Author

dalance commented Apr 29, 2018

I squashed and changed commit message.

@asomers
Copy link
Member

asomers commented Apr 29, 2018

bors r+

bors bot added a commit that referenced this pull request Apr 29, 2018
892: Fixed ptrace::Request cfg conditions r=asomers a=dalance

The cfg condition of ptrace::Request seems to be different from libc.
For example, PTRACE_GETREGS is defined by libc to i686-unknown-linux-gnu target, but it is not defined in ptrace::Request.
I tried to change the cfg condition to the same as libc's condition.
I thinks this change covers the definitions under src/unix/notbsd directory in libc repository.

Co-authored-by: dalance <[email protected]>
@bors
Copy link
Contributor

bors bot commented Apr 29, 2018

@bors bors bot merged commit 1dda866 into nix-rust:master Apr 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants