-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add all definitions from linux/ptp-clock.h #3865
base: main
Are you sure you want to change the base?
Conversation
hmm this needs some extra data types now, and reading kernel C code will need a better-rested brain. I'll continue with this tomorrow. |
e4948dd
to
fab755b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hits a CI error that I don't understand
--- stderr
thread 'main' panicked at /cargo/registry/src/index.crates.io-6f17d22bba15001f/garando_syntax-0.1.1/src/ext/expand.rs:337:21:
internal error: entered unreachable code
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
so that is here https://github.com/JohnTitor/garando/blob/master/garando_syntax/src/ext/expand.rs#L337
but there is no context about what actually failed in this case.
@rustbot review
in the sense that I'm kind of stuck
src/unix/linux_like/linux/mod.rs
Outdated
mod ioctl { | ||
const _IOC_NRBITS: u32 = 8; | ||
const _IOC_TYPEBITS: u32 = 8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this move somewhere else?
I also get a style error on the const definition
src/unix/linux_like/linux/mod.rs:6295: constant found after module when it belongs before
not sure what that is about
Failed to set assignee to
|
1 similar comment
Failed to set assignee to
|
Ah, I don't know anything about this crate but I suspect it will need to be patched somehow. I don't know what that would be (I have never worked with that crate), I know it has problems parsing some newer syntax. Since this is seemingly blocked on that: @rustbot blocked |
If you aren't up for figuring out how to fix that, you could also just try disabling chunks of these changes until things work. Not ideal, but maybe better than waiting. |
well I did just reproduce this locally and that library just performs an out-of-bounds lookup
I'll see what we can do here. My guess is |
Well that's concerning.
Ah, that's a good guess. We have a macro that handles this, search for things with |
btw that out-of-bounds access also happens on the current That also means that I can't actually locally reproduce the CI error unless I start messing with docker (which, I really don't want to do) |
81cd5c7
to
d558fe2
Compare
well, I figured it out. There were really two syntax problems that this crate cannot parse. one issue is the expression for the length of the array pub struct ptp_sys_offset {
pub n_samples: ::c_uint,
pub rsv: [::c_uint; 3],
pub ts: [ptp_clock_time; 2 * PTP_MAX_SAMPLES as usize + 1],
} which gave an error like this
and then also the /// Build an ioctl number, analogous to the C macro of the same name.
const fn _IOC(dir: u32, ty: u32, nr: u32, size: usize) -> u32 {
debug_assert!(dir <= _IOC_DIRMASK);
debug_assert!(ty <= _IOC_TYPEMASK);
debug_assert!(nr <= _IOC_NRMASK);
debug_assert!(size <= (_IOC_SIZEMASK as usize));
(dir << _IOC_DIRSHIFT)
| (ty << _IOC_TYPESHIFT)
| (nr << _IOC_NRSHIFT)
| ((size as u32) << _IOC_SIZESHIFT)
} so, I guess we just use a literal for that length, and I've commented the asserts for now. I am seeing another error that I'm not sure about. These constants are defined like so many of the other constants, so is it more that these constants are not defined in the libc that we're comparing with? They are defined here https://github.com/torvalds/linux/blob/928f79a188aacc057ba36c85b36b6d1e99c8f595/include/uapi/linux/ptp_clock.h#L234 and don't seem special otherwise.
also there is some musl stuff that I'll need to figure out. Anyway, progress! |
@tgross35 how do you recommend implementing this? cfg_if! {
if #[cfg(any(target_arch = "sparc", target_arch = "sparc64"))] {
s!{
pub struct ptp_clock_caps {
pub max_adj: ::c_int,
pub n_alarm: ::c_int,
pub n_ext_ts: ::c_int,
pub n_per_out: ::c_int,
pub pps: ::c_int,
pub n_pins: ::c_int,
pub cross_timestamping: ::c_int,
pub adjust_phase: ::c_int,
pub rsv: [::c_int; 12],
}
}
} else if #[cfg(any(target_env = "musl", target_env = "ohos"))] {
s!{
pub struct ptp_clock_caps {
pub max_adj: ::c_int,
pub n_alarm: ::c_int,
pub n_ext_ts: ::c_int,
pub n_per_out: ::c_int,
pub pps: ::c_int,
pub n_pins: ::c_int,
pub cross_timestamping: ::c_int,
pub rsv: [::c_int; 13],
}
}
} else {
s! {
pub struct ptp_clock_caps {
pub max_adj: ::c_int,
pub n_alarm: ::c_int,
pub n_ext_ts: ::c_int,
pub n_per_out: ::c_int,
pub pps: ::c_int,
pub n_pins: ::c_int,
pub cross_timestamping: ::c_int,
pub adjust_phase: ::c_int,
pub max_phase_adj: ::c_int,
pub rsv: [::c_int; 11],
}
}
}
} it runs into a lint of having more than one |
@rustbot review |
☔ The latest upstream changes (presumably #3480) made this pull request unmergeable. Please resolve the merge conflicts. |
Co-authored-by: Trevor Gross <t.gross35@gmail.com>
adopted from RFL
cf84cec
to
a458ded
Compare
code is the same as #3559, but rebased on
main
instead of0.2
r? @tgross35