retep998 / wio-rs

Windows things for Rust
Apache License 2.0
52 stars 14 forks source link

Creating ComPtr from APIs that take an out-parameter #10

Open SimonSapin opened 6 years ago

SimonSapin commented 6 years ago

I’ve added the API below through an extension trait. Do you think it would make sense to have as inherent methods in this crate? I can send a PR. I’m not sure about the method names, though.

impl<T: Interface> ComPtr<T> {
    /// For use with APIs that "return" a new COM object through a `*mut *mut c_void` out-parameter.
    ///
    /// # Safety
    ///
    /// `T` must be a COM interface that inherits from `IUnknown`.
    /// If the closure makes the inner pointer non-null,
    /// it must point to a valid COM object that implements `T`.
    /// Ownership of that object is taken.
    unsafe fn from_void_out_param<F>(f: F) -> Result<Self, HRESULT>
        where F: FnOnce(*mut *mut c_void) -> HRESULT
    {
        Self::from_out_param(|ptr| f(ptr as _))
    }

    /// For use with APIs that "return" a new COM object through a `*mut *mut T` out-parameter.
    ///
    /// # Safety
    ///
    /// `T` must be a COM interface that inherits from `IUnknown`.
    /// If the closure makes the inner pointer non-null,
    /// it must point to a valid COM object that implements `T`.
    /// Ownership of that object is taken.
    unsafe fn from_out_param<F>(f: F) -> Result<Self, HRESULT>
        where F: FnOnce(*mut *mut T) -> HRESULT
    {
        let mut ptr = ptr::null_mut();
        let status = f(&mut ptr);
        if SUCCEEDED(status) {
            Ok(ComPtr::from_raw(ptr))
        } else {
            if !ptr.is_null() {
                let ptr = ptr as *mut IUnknown;
                (*ptr).Release();
            }
            Err(status)
        }
    }
}
cranflavin commented 6 years ago

Interesting approach. I'd suggest checking explicitly against S_OK in from_out_param. COM APIs are known to return S_FALSE sometimes without actually filling in the out param. In that case the caller should probably treat S_FALSE like an error and handle it differently.

CarePackage17 commented 5 years ago

Concerning the naming, C++/WinRT uses put().

SimonSapin commented 5 years ago

That’s not quite the same. That put() method doen’t do the whole “create a (null) pointer, then mutate it through a callback, then check whether return value indicates an error” dance.