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

Moved ptrace constants into enum types plus minor additions #749

Merged
merged 3 commits into from
Aug 28, 2017
Merged

Moved ptrace constants into enum types plus minor additions #749

merged 3 commits into from
Aug 28, 2017

Conversation

xd009642
Copy link
Contributor

Reopening of #734 hence the branch name, changelog will be updated once the pr number of this new pull request is known.

@xd009642
Copy link
Contributor Author

Also @Susurrus rust-lang/libc#745 (comment) is currently been tested by bors. As this crate uses the latest version of libc off github is it worth waiting to see if it gets merged before we close this?

@xd009642
Copy link
Contributor Author

And it's been merged, that means I can remove the casts if you're happy with that?

@Susurrus
Copy link
Contributor

Yeah, let's wait on that. Then we can remove the casting from this PR. I'd like to keep the libc_enum! changes tho, as they might be useful for others. Eventually we'll be removing those macros, but for now it's worth keeping.

@xd009642
Copy link
Contributor Author

Right removed cast and updated changelog. Should be all good unless I've missed something

CHANGELOG.md Outdated
@@ -47,6 +49,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#731](https://github.com/nix-rust/nix/pull/731))
- Marked `pty::ptsname` function as `unsafe`
([#744](https://github.com/nix-rust/nix/pull/744))
- Ability to cast types added to libc_enum!
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an internal API change and as such should not be documented in the CHANGELOG.

pid,
ptr::null_mut(),
ptr::null_mut()
).map(|_| ())
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be outdented one.

/// Detaches the current running process, as with `ptrace(PTRACE_DETACH, ...)`
///
/// Detaches from the process specified in pid allowing it to run freely
pub fn detach(pid: Pid) -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move the addition of this function into a separate commit following the existing two and give it a separate entry under "Added" in the CHANGELOG.

CHANGELOG.md Outdated
@@ -22,6 +22,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#739](https://github.com/nix-rust/nix/pull/739))
- Expose `signalfd` module on Android as well.
([#739](https://github.com/nix-rust/nix/pull/739))
- Added enums for ptrace request, event and options to replace constants.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be under "Changed", since we changed a public API, we didn't add a new one really. I would say something like "Changed the PtraceEvent, PtraceOptions, and PtraceRequest datatypes into enums and updated ptrace function argument types accordingly."

Used the libc_enum! macro to create enums for the ptrace event, request, and libc_bitflags for options constants defined in libc.
Also, replicated functionality to move from c_int to PtraceEvent enum in PR #728 as it appears to be abandoned.

Added utility function for detaching from tracee. Updated names and removed ptrace::ptrace namespace
@xd009642
Copy link
Contributor Author

Changelog changes addressed and detach moved to a separate commit and added to the changelog!

@Susurrus
Copy link
Contributor

@xd009642 Looks good to me! Thanks for all your hard work on this.

bors r+

bors bot added a commit that referenced this pull request Aug 28, 2017
749: Moved ptrace constants into enum types plus minor additions r=Susurrus a=xd009642

Reopening of #734 hence the branch name, changelog will be updated once the pr number of this new pull request is known.
@bors
Copy link
Contributor

bors bot commented Aug 28, 2017

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