ibm-s390-linux / s390-tools

Tools for use with the s390 Linux kernel and device drivers
MIT License
62 stars 58 forks source link

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

Open sharkcz opened 8 months ago

sharkcz commented 8 months ago

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 commented 7 months ago

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