Skip to content

Add structs defined in linux/input.h #616

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

Merged
merged 3 commits into from
Jun 25, 2017

Conversation

Arvamer
Copy link
Contributor

@Arvamer Arvamer commented Jun 13, 2017

I was using definitions for these structs from ioctl but because @cmr decide to deprecate his crate (and yanked all versions :() I think that libc is the best place for them.

In original C header, primitive types uses aliases like __u16 or __s32; for now I replaced them with Rust's counterparts but I'm not sure if it is ok, especially because tests were failing for u64 (unsigned long long vs unsigned long on x86_64). Also, should I do something special with union in ff_effect?

I would like also to add all constants form linux/input.h and linux/input-event-codes.h if this PR will be accepted.

@Arvamer Arvamer force-pushed the linux_input_types branch from a9ccb11 to 2f8a5b0 Compare June 13, 2017 16:25
@Arvamer
Copy link
Contributor Author

Arvamer commented Jun 13, 2017

So mips64 and powerpc64 doesn't like c_ulonglong == __u64 while x86_64 (and probably aarch64) are against u64 == __u64.

And musl targets don't have linux/input.h – but shouldn't this come from kernel package? At least on Archlinux, it's owned by linux-api-headers.

@alexcrichton
Copy link
Member

Ah there's an issue about what to do for linux/*.h which has what I remember as some scary warnings about including linux/*.h headers? Not all of them are apparently intented for consumption in user-space?

@Arvamer
Copy link
Contributor Author

Arvamer commented Jun 14, 2017

I would be surprised if linux/input.h wasn't intended for consumption in user-space. It's the only place where types for working with evdev are defined, and for example SDL2 include it directly.

I also tried running test suite locally with CC=musl-gcc HOST=x86_64-unknown-linux-musl and it works.

@alexcrichton
Copy link
Member

I personally don't know much about linux header files and whether or not they should be included. This can't merge until the CI is green though, and I don't have a particular preference on how that's done.

@Arvamer Arvamer force-pushed the linux_input_types branch 3 times, most recently from 25344a4 to 941d897 Compare June 14, 2017 18:45
@Arvamer Arvamer force-pushed the linux_input_types branch from 941d897 to 60d9322 Compare June 14, 2017 19:48
@Arvamer
Copy link
Contributor Author

Arvamer commented Jun 15, 2017

CI is green now.

@alexcrichton
Copy link
Member

@bors: r+

Looks great, thanks!

@bors
Copy link
Contributor

bors commented Jun 15, 2017

📌 Commit 92fac1d has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jun 15, 2017

🔒 Merge conflict

@bors
Copy link
Contributor

bors commented Jun 15, 2017

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

@emberian
Copy link
Member

emberian commented Jun 15, 2017 via email

@Arvamer
Copy link
Contributor Author

Arvamer commented Jun 15, 2017

I still think that these structs (and constants) should be defined in libc. evdev crate looks nice and I will probably switch to using it, but it's on higher level than libc, and at least for me, libc looks like logical place when you are looking for these definitions.

@Susurrus
Copy link
Contributor

Was there consensus on what to do with this?

@alexcrichton
Copy link
Member

Yes this is fine, sorry for the delay! @Arvamer feel free to ping me whenever a PR is updated, unfortunately GitHub doesn't send out notifications for that :(

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 25, 2017

📌 Commit 495d22a has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jun 25, 2017

⌛ Testing commit 495d22a with merge a38631f...

bors added a commit that referenced this pull request Jun 25, 2017
Add structs defined in linux/input.h

I was using definitions for these structs from `ioctl` but because @cmr decide to deprecate his crate (and yanked all versions :() I think that `libc` is the best place for them.

In original C header, primitive types uses aliases like `__u16` or `__s32`; for now I replaced them with Rust's counterparts but I'm not sure if it is ok, especially because tests were failing for `u64` (`unsigned long long` vs `unsigned long` on x86_64). Also, should I do something special with union in `ff_effect`?

I would like also to add all constants form `linux/input.h` and `linux/input-event-codes.h` if this PR will be accepted.
@bors
Copy link
Contributor

bors commented Jun 25, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing a38631f to master...

@bors bors merged commit 495d22a into rust-lang:master Jun 25, 2017
danielverkamp pushed a commit to danielverkamp/libc that referenced this pull request Apr 28, 2020
These aren't actually used anywhere yet, but this is likely a first step
in any world! Also note that these types are unstable currently.
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