windows-rs: Functions that set last error could return `Result`

You can do it for the different return types as long as the function metadata has SetLastError=true. Here’s my suuuuuuper simple implementation. I think you would need to change it to return a Result<T, result::error>, and then re-implement HRESULT_FROM_WIN32 in Rust accordingly:

pub trait Win32ReturnIntoResult
where
    Self: Sized,
{
    fn into_result(self) -> eyre::Result<Self>;
}

impl Win32ReturnIntoResult for BOOL {
    fn into_result(self) -> eyre::Result<BOOL> {
        match self.as_bool() {
            false => Err(std::io::Error::last_os_error().into()),
            _ => Ok(self),
        }
    }
}

impl Win32ReturnIntoResult for HANDLE {
    fn into_result(self) -> eyre::Result<HANDLE> {
        match self.is_invalid() {
            true => Err(std::io::Error::last_os_error().into()),
            _ => Ok(self),
        }
    }
}

impl Win32ReturnIntoResult for HHOOK {
    fn into_result(self) -> eyre::Result<HHOOK> {
        match self.is_null() {
            true => Err(std::io::Error::last_os_error().into()),
            _ => Ok(self),
        }
    }
}

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 1
Comments: 19 (10 by maintainers)

Most upvoted comments

The CLR provides special handling here since code may execute following the API call that would invalidate the last error stored on the thread, but this is not a problem that languages like C++ or Rust have to deal with. Keep in mind that the win32 metadata is not specific to .NET but is meant to convey information about APIs for all languages. Anyway, @sotteson1 can confirm what it is meant to indicate.

SetLastError means the API calls SetLastError, as per the docs for that API. Language projections can do what they want with that bit of information, including ignoring it.

sotteson1 on Sep 13, 2021

@riverar Nothing I see about that file is super encouraging to it being a fuzzy scanner 😄

https://github.com/microsoft/win32metadata/issues/537 https://github.com/microsoft/win32metadata/commit/0992cabd9f7d44c5375d133a44ddf18c17a4d694 https://github.com/microsoft/win32metadata/commit/b90be05f2911ce2af6aa3842390109b2d237ba44

A good intern project might be to use a sentiment detector on the documentation, but otherwise you definitely would need something like what @kennykerr just mentioned for what the success and failure value are so that you could intelligently convert to an Error when necessary. Then again, this might be a great opportunity to unify the return value documentation so that it’s consistent and easy to parse.

When I opened the ticket I hadn’t run across the horrors of multi-valued BOOLs yet. They’re just true and false, right? right? riiiiiight…

ryancerium on Sep 10, 2021

@riverar The core issue with repurposing the SetLastError attribute is, that it lacks semantic information. In case of GetFileSize, the result of calling GetLastError is only meaningful if the function returned 0xFFFF_FFFF. The issue isn’t about ergonomics but correctness.

Ryan provided good examples. Likewise complex as the latter one is EnumWindows, where the implementer of the callback procedure is responsible for setting the error code in case of early termination.

The SetLastError attribute really has a single use-case: Instruct the CLR marshaler to cache the last error code so that managed code can later access it, if it needs to. A boolean attribute is sufficient for this purpose. It’s rather insufficient as a base for more complex decisions (which would be needed here).

tim-weis on Sep 10, 2021

A better example is GetMessageW which returns a BOOL that is 0 if it was a WM_QUIT message, -1 on error, and anything else on success. Those are not the queen’s booleans there.

GetWindowLongPtrW and GetWindowTextLengthW also have slightly more complex semantics where a 0 return may be valid, but you have to call SetLastError() beforehand and GetLastError() afterwards to find out.

ryancerium on Sep 9, 2021