linux: Use the input_event_X macros to access input_event members #4654
Conversation
This means the same members are available in the struct no matter whether it is a 32 or 64 bit platform or the linux_time_bits64 config is set.
tgross35
left a comment
tgross35
left a comment
There was a problem hiding this comment.
I'm sorry this has taken me forever to come to consensus on, but I do now think this is the right approach. I have a couple of non-technical requests, the code changes LGTM.
| // FIXME(1.0): Change to the commented variant, see https://github.com/rust-lang/libc/pull/4148#discussion_r1857511742 | ||
| #[cfg(any(target_pointer_width = "64", not(linux_time_bits64)))] | ||
| pub time: crate::timeval, | ||
| // #[cfg(any(target_pointer_width = "64", not(linux_time_bits64)))] | ||
| // pub input_event_sec: time_t, | ||
| // #[cfg(any(target_pointer_width = "64", not(linux_time_bits64)))] | ||
| // pub input_event_usec: suseconds_t, | ||
| // #[cfg(target_arch = "sparc64")] | ||
| // _pad1: c_int, | ||
| pub input_event_sec: crate::time_t, | ||
| #[cfg(all(target_pointer_width = "32", linux_time_bits64))] | ||
| pub input_event_sec: c_ulong, |
There was a problem hiding this comment.
Could you add a comment that these are actually combined as timeval?
| #[cfg(target_arch = "sparc64")] | ||
| _pad1: c_int, |
There was a problem hiding this comment.
I would like to backport this part of the change. It is technically breaking but our definition is incorrect without it, and sparc64 isn't all that popular of a target. Would you be able to split this to a separate commit?
Padding fields should also get wrapped in Padding<...> now
There was a problem hiding this comment.
Hm - actually, I think this change may not be correct on its own. Looks like the __usec/input_event_usec field would also need to be changed from a long to an int https://github.com/torvalds/linux/blob/3f9f0252130e7dd60d41be0802bf58f6471c691d/include/uapi/linux/input.h#L35C6-L42.
We should probably do that change still to make sure users don't expect the upper half of that field to work, just annoying that sparc has a special-cased suseconds_t in the first place.
(context: torvalds/linux@2e74694)
There was a problem hiding this comment.
Nevermind, I see now that the field is of type suseconds_t. Sorry for the confusion; please disregard everything in this thread except for the Padding<...> bit.
|
Reminder, once the PR becomes ready for a review, use |
|
I've seen your comments and hope to get around to this next week. |
3 participants
Description
Use the
input_event_secandinput_event_usecmembers (macros in C) on all platforms, no matter whethertime_tis 32 or 64 bits.This means the same members are available in the struct no matter whether it is a 32 or 64 bit platform or the linux_time_bits64 config is set.
This is a breaking change.
Sources
https://github.com/torvalds/linux/blob/0cc53520e68bea7fb80fdc6bdf8d226d1b6a98d9/include/uapi/linux/input.h#L28
Checklist
libc-test/semverhave been updated*LASTor*MAXareincluded (see #3131)
cd libc-test && cargo test --target mytarget);especially relevant for platforms that may not be checked in CI
This is a breaking change and should not be backported to the 0.2 branch.