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.

Sign up for GitHub

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

Jump to bottom

Expose signal value of siginfo_t #1421

Merged
merged 1 commit into from
Jul 9, 2019
Merged

Expose signal value of siginfo_t #1421

merged 1 commit into from
Jul 9, 2019

Conversation

DoumanAsh
Copy link
Contributor

@DoumanAsh DoumanAsh commented Jul 4, 2019 •
edited
Loading

Currently exposes it for following platforms:

  • Linux has it as enum _timer and it is exposed alongside sigval
  • On FreeBSD like systems si_value follows after si_addr (according to headers)
  • Darwin is similar to FreeBSD
  • NetBSD and FreeBSD uses struct that contains pid, uid, and si_value. Exposed via it

P.s. I'd like to take a look at other platforms too, but these are what I know so far.
FreeBSD needs some testing though

cc @gnzlbg

@rust-highfive
Copy link

r? @gnzlbg

(rust_highfive has picked a reviewer for you, use r? to override)

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 4, 2019

Thank you for the PR. Our CI is currently down because we are migrating to Azure Pipelines, but it looks good, I'll try to merge it this weekend.

@DoumanAsh
Copy link
Contributor Author

Updated with other unix platforms.
I used publicly available repositories with source code as reference.
But I'm not sure how correct it is per se

@gnzlbg gnzlbg closed this Jul 7, 2019
@gnzlbg gnzlbg reopened this Jul 7, 2019
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 7, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 7, 2019

📌 Commit 2cd9b40 has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Jul 7, 2019

⌛ Testing commit 2cd9b40 with merge 2a58378...

bors added a commit that referenced this pull request Jul 7, 2019
@bors
Auto merge of #1421 - DoumanAsh:expose_posix_timer_value, r=gnzlbg
2a58378 
Expose signal value of siginfo_t

Currently exposes it for following platforms:

- Linux has it as enum _timer and it is exposed alongside sigval
- On FreeBSD like systems si_value follows after si_addr (according to headers)
- Darwin is similar to FreeBSD
- NetBSD and FreeBSD uses struct that contains pid, uid, and si_value. Exposed via it

P.s. I'd like to take a look at other platforms too, but these are what I know so far.
FreeBSD needs some testing though

cc @gnzlbg
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 7, 2019

@asomers @semarie this might need tweaking for OpenBSD, but it shouldn't break anything to land this - i'm not sure about NetBSD, but with this in master it should be easier to try this out, and improve this iteratively.

@bors
Copy link
Contributor

bors commented Jul 7, 2019

💔 Test failed - checks-cirrus-freebsd-11

@asomers
Copy link
Contributor

asomers commented Jul 7, 2019

You need to double check the size of your pad fields. They aren't right for 64-bit FreeBSD.

@DoumanAsh
Copy link
Contributor Author

Ok, I'll need to check it.
I was using signal.h as reference but i didn't exactly check every type inside.
Wonder if it is sigval union behaving

@DoumanAsh DoumanAsh force-pushed the expose_posix_timer_value branch 2 times, most recently from cb112c9 to 80f8829 Compare July 7, 2019 17:10
@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 9, 2019

It appears that the freebsd jobs are still failing.

@DoumanAsh
Copy link
Contributor Author

@gnzlbg Yeah, before my traveling I actually was trying to fix MacOS tests, which are still failing for some reason.
But as for freebsd I'm not exactly sure even where it fails, so I'd need to take a look at how this error ouput gets generated

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 9, 2019

you might just need to rebase on top of master, those error shouldn't be there, and don't appear related to this PR

@DoumanAsh
Copy link
Contributor Author

DoumanAsh commented Jul 9, 2019 •
edited
Loading

Ok, done, I'll check it out once CI is finished

UPD: Removed pub from internal struct in apple module.
CI should be working now

@DoumanAsh
Expose signal value of siginfo_t
0c2e783 
Exposes value for most unix like platforms
@DoumanAsh
Copy link
Contributor Author

@gnzlbg CI passed

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 9, 2019

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 9, 2019

📌 Commit 0c2e783 has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented Jul 9, 2019

⌛ Testing commit 0c2e783 with merge 6307a0b...

bors added a commit that referenced this pull request Jul 9, 2019
@bors
Auto merge of #1421 - DoumanAsh:expose_posix_timer_value, r=gnzlbg
6307a0b 
Expose signal value of siginfo_t

Currently exposes it for following platforms:

- Linux has it as enum _timer and it is exposed alongside sigval
- On FreeBSD like systems si_value follows after si_addr (according to headers)
- Darwin is similar to FreeBSD
- NetBSD and FreeBSD uses struct that contains pid, uid, and si_value. Exposed via it

P.s. I'd like to take a look at other platforms too, but these are what I know so far.
FreeBSD needs some testing though

cc @gnzlbg
@bors
Copy link
Contributor

bors commented Jul 9, 2019

☀️ Test successful - checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing 6307a0b to master...

@bors bors merged commit 0c2e783 into rust-lang:master Jul 9, 2019
@bors bors mentioned this pull request Jul 9, 2019
Closed
@DoumanAsh DoumanAsh deleted the expose_posix_timer_value branch July 9, 2019 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gnzlbg gnzlbg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@DoumanAsh @rust-highfive @gnzlbg @bors @asomers