I think you're right. My mistake. Maybe the best option is `&mut windows_sys::core::BSTR` (using the unsafe BSTR type I mentioned), since that way the same BSTR type can be used for the two arguments. Or `*mut BSTR`, since windows-rs itself seems to prefer raw pointers for whatever reason, though I found crates using windows-rs that seem to successfully use references.

I am slightly suspicious that Raymond Chen might have been confused. The link in that post has the text "forget to set the output pointer to NULL", but in the linked post (the original link is broken but it's at [1]), the implementation actually set the output pointer to a garbage value rather than leaving it untouched. I wonder what the marshalling implementation actually looks like…

But at any rate, treating the out pointer as uninitialized is definitely the safe option. I'm not 100% sure whether it can legitimately point to non-null, but if it does point to non-null, then that value is definitely garbage rather than something that needs freeing.

[1] https://devblogs.microsoft.com/oldnewthing/20091007-00/?p=16...

▲ garaetjjte 4 days ago | parent [-]

>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.

▲ piker 4 days ago | parent [-]

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 | parent [-]

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

	▲	piker 2 days ago \| parent [-]
		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.