← Back to context

Comment by garaetjjte

4 days ago

>I think you're right. My mistake.

I didn't disagree with you, I just wanted to point another issue.

Actually *mut BSTR (owned) is also acceptable, iff you remember to use std::ptr::write instead of normal assignment.

> I'm not 100% sure whether it can legitimately point to non-null

Note that in none of the examples on this and other posts (like https://devblogs.microsoft.com/oldnewthing/20040326-00/?p=40...) output value is initialized, so it will be whatever is lying on the stack.

3 comments

garaetjjte

Reply
piker  4 days ago

These are all great points, and I will update the blog post to reflect them in the morning.

I believe this approach can work while retaining the most apparently-idiomatic mapping. What do you guys think?

impl IRibbonExtensibility_Impl for Addin_Impl {

    unsafe fn GetCustomUI(&self, _ribbon_id: BSTR, out: *mut BSTR) -> HRESULT {

        log("GetCustomUI called()");

        std::mem::forget(_ribbon_id);

        if out.is_null() {

            return windows::Win32::Foundation::E_POINTER;
        }

        unsafe {

            std::ptr::write(out, BSTR::from(RIBBON_XML));
        }

        S_OK
    }

}

  • garaetjjte  3 days ago

    Looks fine to me, if you're avoiding wrappers like ManuallyDrop/MaybeUninit.

    • piker  3 days ago

      Actually the `windows-rs` team weighed in:

      impl IRibbonExtensibility_Impl for Addin_Impl {

          unsafe fn GetCustomUI(

        &self,

        _ribbon_id: windows::core::Ref<BSTR>,

        out: windows::core::OutRef<BSTR>,

    ) -> HRESULT {

        log("GetCustomUI called()");

        if out.is_null() || out.write(BSTR::from(RIBBON_XML)).is_err() {

            return E_POINTER;

        };

        S_OK

    }

      }

      https://github.com/microsoft/windows-rs/issues/3832

      Thanks for pushing on the issue! I've updated the blog post for GetCustomUI.