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

wait: support ptrace events for Linux #273

Closed
wants to merge 1 commit into from

Conversation

abbradar
Copy link
Contributor

Part of #270 -- see abbradar@b2d3b4a for previous discussion.

@kamalmarhubi
Copy link
Member

Copying part of my comment on the previous discussion:

I'm also a bit put off by adding a meaningless member on non-Linux systems. Trying to think of alternatives...

I'm going to hold off on this for a bit while I think about stuff related to #190. :-)

@homu
Copy link
Contributor

homu commented Jun 27, 2016

☔ The latest upstream changes (presumably #362) made this pull request unmergeable. Please resolve the merge conflicts.

@kamalmarhubi
Copy link
Member

Coming back to this, I'd be happier if the extra variant and methods were cfg'ed to only be present on Linux, or whichever systems support it.

@luser
Copy link

luser commented Oct 13, 2016

I was thinking about this again, and how to handle this in a way that isn't terrible across platforms. What would you think about adding another entry to the WaitStatus enum, like WaitStatus::PtraceEvent? Then the if status::stopped case could check for this and return that value. It'd still be useless on non-Linux systems, but it seems like it'd be easier to work with a platform-conditional enum variant than a conditional field inside an enum variant. #[cfg(...)] seems to work OK on enum variants and match arms:
https://play.rust-lang.org/?gist=15519135f09722bf6d8653912592c079&version=stable&backtrace=0

@abbradar
Copy link
Contributor Author

Sigh, I still haven't returned to this (haven't actually got a chance to touch Rust for a long time already). I understand that this can be frustrating, and also -- if anyone wants to continue this work please do so!

I hope I can return to this someday...

@kamalmarhubi
Copy link
Member

@abbradar this is not frustrating at all! And thanks for your contributions :-)

@kamalmarhubi
Copy link
Member

@luser

What would you think about adding another entry to the WaitStatus enum, like WaitStatus::PtraceEvent?

Could you sketch out what you mean a tiny bit?

@luser
Copy link

luser commented Nov 4, 2016

Oh, I can do you one better! I actually wrote the patch because I wanted to use it for something, I just forgot to do anything with it. Here it is:
luser@92a3cf7

Here's the code I wrote that uses it:
https://github.com/luser/tracetree/blob/master/src/main.rs#L65

I didn't actually try to make that code portable yet, but making it build should be straightforward. (Making it actually work right is probably harder).

homu added a commit that referenced this pull request Feb 14, 2017
wait: Support ptrace events for Linux

Adds new WaitStatus value `PtraceEvent`. Implementation of #273 that only affects Linux/Android.
@Susurrus
Copy link
Contributor

Susurrus commented Jun 2, 2017

Is this still relevant? If so, @abbradar would you be willing to update this patch?

@luser
Copy link

luser commented Jun 2, 2017

This was superseded by #438

You can see an example of using it here: https://github.com/luser/tracetree/blob/2dcde59a9b2bd8dd6343a147a6d09f5d4428068e/src/main.rs#L141

@Susurrus
Copy link
Contributor

Susurrus commented Jun 2, 2017

Okay, then I'm going to close this. Thanks, @luser.

@Susurrus Susurrus closed this Jun 2, 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.

5 participants