Am 12.02.2025 um 14:47 hat Paolo Bonzini geschrieben: > On Wed, Feb 12, 2025 at 2:13 PM Kevin Wolf <kw...@redhat.com> wrote: > > Yes, we definitely need some proper bindings there. I'm already tired of > > writing things like this: > > > > return -(bindings::EINVAL as std::os::raw::c_int) > > > > Or even: > > > > return e > > .raw_os_error() > > .unwrap_or(-(bindings::EIO as std::os::raw::c_int)) > > This by the way seemed incorrect to me as it should be > > return -(e > .raw_os_error() > .unwrap_or(bindings::EIO as std::os::raw::c_int)) > > (leaving aside that raw_os_error does not work on Windows)... But then > I noticed that read_raw() also does not negate, which causes the error > to print incorrectly...
Yes, I actually noticed that after sending the email and fixed it (and another instances of the same) up locally. > > Which actually already shows that your errno binding patch does the > > opposite direction of what I needed in this series. > > ... so my patch already helps a bit: you can still change > > if ret < 0 { > Err(Error::from_raw_os_error(ret)) > } else { > Ok(()) > } > > to > > errno::into_io_result(ret)?; > Ok(()) > > and avoid the positive/negative confusion. No reason to write essentially if (ret != 0) { ret } else { 0 }. This one should do: errno::into_io_result(ret) And yes, it's already a good improvement. > Anyhow, I guess the first one wouldn't be much better: > > return errno::into_negative_errno(ErrorKind::InvalidInput); > > whereas the second could be > > return errno::into_negative_errno(e); > > But then the first is already a special case; it only happens where > your bindings are not trivial thanks to the introduction of the > Mapping type. Yes, the second one seems like the more important one because the other one should only happen in bindings eventually. We could still have something like an errno!(InvalidInput) to make the code in bindings less verbose. Or if you have to define the constants anyway - you currently do this only for Windows, but for into_negative_errno() you might need it on Linux, too - and it wouldn't be a problem for the constants to be signed (that they are unsigned is the main reason why it becomes so ugly with the bindgen constants), you could just make it -errno::EINVAL again. Kevin