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

rust/pv/src/uvdevice.rs fails to compile on i686 #159

Open
sharkcz opened this issue Nov 9, 2023 · 1 comment
Open

rust/pv/src/uvdevice.rs fails to compile on i686 #159

sharkcz opened this issue Nov 9, 2023 · 1 comment

Comments

@sharkcz
Copy link
Contributor

sharkcz commented Nov 9, 2023

We are not planning to build officially the "cloud" tools for i686, but my test build failed with

...
error[E0308]: mismatched types
   --> /builddir/build/BUILD/s390-tools-2.29.0/rust/pv/src/uvdevice.rs:196:39
    |
196 |         ioctl_raw(self.0.as_raw_fd(), cmd.cmd(), &mut cb)?;
    |         ---------                     ^^^^^^^^^ expected `u32`, found `u64`
    |         |
    |         arguments to this function are incorrect
    |
note: function defined here
   --> /builddir/build/BUILD/s390-tools-2.29.0/rust/pv/src/uvdevice.rs:42:4
    |
42  | fn ioctl_raw(raw_fd: RawFd, cmd: c_ulong, cb: &mut IoctlCb) -> Result<()> {
    |    ^^^^^^^^^                ------------
help: you can convert a `u64` to a `u32` and panic if the converted value doesn't fit
    |
196 |         ioctl_raw(self.0.as_raw_fd(), cmd.cmd().try_into().unwrap(), &mut cb)?;
    |                                                ++++++++++++++++++++
For more information about this error, try `rustc --explain E0308`.
error: could not compile `pv` (lib) due to previous error

please see logs from https://koji.fedoraproject.org/koji/taskinfo?taskID=108797994 for all details

Such type mismatch errors often expose some real issues, so please review if it is something we would like to fix.

@steffen-eiden
Copy link
Member

The code implicitly assumes that a c_ulong = in C a long int is 64 bit wide. This is true for any 64bit arch but not for a 32bit arch.
Do you see any deeper problem with that?

I could fix it to assume 32bit for a 32bit arch, but I am not sure if that's worth the effort. Easy solution: cast to u32 as the value is 32bit wide anyways.

We could also add a note that we do not support 32bit (and fail compiling if there is an easy way).

Anyhow, this code should never be used on a non s390-machine and we support 64bit only AFAIK.
I avoided using arch barriers there to reduce complexity on the pv library crate.

Any suggestions?

Thanks
Steffen

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

No branches or pull requests

2 participants