Using BSTR from a borrowed pointer

[ohadravid]

Suggestion

Hi,

Recently I wanted to use windows_strings::BSTR, but didn't have ownership of the original pointer.

Is there a way to have something like BSTR::from_raw_borrowed(*const u16) -> &BSTR?

Background

While updating the wmi-rs crate to windows version 0.58 (PR), there was a need to change this code:

// in a `fn from_variant(vt: &VARIANT)`
let variant_value = match variant_type {
    VT_BSTR => {
        let bstr_ptr: &BSTR = unsafe { &vt.Anonymous.Anonymous.Anonymous.bstrVal };

        Variant::String(bstr_ptr.try_into()?)
    }
    ...
}

Since bstrVal is *const u16.

The result is something like this:

let bstr_ptr = unsafe { BSTR::from_raw(vt.Anonymous.Anonymous.Anonymous.bstrVal) };
let bstr_as_str = bstr_ptr.to_string();
// We don't want to be the ones freeing the BSTR.
let _ = bstr_ptr.into_raw();

(Which I think is right, since the VARIANT's data is owned by someone else)

This would have been more ergonomic and less error prune if where was an option to construct a &BSTR from the raw pointer.

Thanks for the awesome work!

[tim-weis]

BSTR::from_raw_borrowed(*const u16) -> &BSTR cannot exist, since pointers aren't associated with lifetimes.

If you need an owned copy of a BSTR call SysAllocString to create a copy.

[ohadravid]

since pointers aren't associated with lifetimes

BSTR::from_raw_borrowed(&*const u16) -> &BSTR would also work in this case, similar to Interface::from_raw_borrowed

[tim-weis]

The code you've shown reads like you're looking for ManuallyDrop. This wrapper allows you to construct a BSTR from a pointer owned elsewhere, without accidentally running its destructor.

As far as functionality goes, what you're asking for is impl TryFrom<&VARIANT> for String, unless I'm misunderstanding the code. I suppose that could be implemented, although I'd still like to have a better string type for the windows family of crates, instead of baking the String/str/OsString/OsStr/CString/CStr conversion machinery into every interface.

[kennykerr]

although I'd still like to have a better string type for the windows family of crates

HSTRING should be that string type. If its not let me know. 😊

[tim-weis]

HSTRING should be that string type. If its not let me know. 😊

HSTRING is a great example of what I'm looking for. It serves as a canonical string representation, that consolidates conversions from and to other string types. It's a surprisingly simple design that works very well in practice.

The string type I'm looking for is like HSTRING with a few extras: It needs to be mutable and "pre-allocatable". The intended use is as the input/output area for API calls. The primary goal is to losslessly maintain (UTF-16) strings while passing them through a sequence of API calls.

However, I should dedicate a separate issue to this instead of hijacking this one.

[kennykerr]

You may want to consider HStringBuilder.

[ohadravid]

Hi @kennykerr - WDYT about having a BSTR::from_raw_borrowed(&*const u16) -> &BSTR? (the original issue / feature request is unrelated to HSTRING and "better string" stuff).

Sounds to me like this should be safe (and useful) 😄

[tim-weis]

BSTR::from_raw_borrowed(&*const u16) -> &BSTR is foot gun material. A BSTR is a *const u16 with additional properties (namely, a length prefix and its corollaries). Failure to encode that in the type system will inevitably break on either side of the (non-)contract.

Since you want a &[u16] only you don't need to construct a BSTR at all. You know the pointer to the start already (bstrVal) and can discover the length using SysStringLen. This information can be passed into slice::from_raw_parts.

If that is not something you want to do you could also transmute a reference to bstrVal into a &BSTR (I think), and get a borrowed BSTR for its interface alone.

[ohadravid]

@tim-weis What I'm asking is if it makes sense that BSTR would provide this transmute since it is a useful thing to provide (IMO).

For example, if I did this transmute before and BSTR wasn't repr(transparent) I would have gotten UB in my crate.

Your other suggestions don't really apply for my usecase.

[kennykerr]

I think there's a larger issue here around the usability/ergonomics of BSTR/VARIANT/SAFEARRAY and I've attempted to make it easier to work with BSTR and VARIANT but there's still clearly room for improvement.

[kennykerr]

Related: #2983

[tim-weis]

Your other suggestions don't really apply for my usecase.

If you think that transmuting applies, then the other suggestion applies just as well. Constructing a slice manually merely avoids relying on BSTR being repr(transparent), but the code generated is identical. This is how to_wide() is implemented¹:

windows-rs/crates/libs/strings/src/bstr.rs

Lines 30 to 33 in 9f5ec21

	/// Get the string as 16-bit wide characters (wchars).
	pub fn as_wide(&self) -> &[u16] {
	unsafe { core::slice::from_raw_parts(self.as_ptr(), self.len()) }
	}

And this this len():

windows-rs/crates/libs/strings/src/bstr.rs

Lines 21 to 28 in 9f5ec21

	/// Returns the length of the string.
	pub fn len(&self) -> usize {
	if self.0.is_null() {
	0
	} else {
	unsafe { bindings::SysStringLen(self.0) as usize }
	}
	}

Either approach works, just as wrapping the BSTR inside a ManuallyDrop, and either of these is more resilient when the code gets changed than what you have now.

This is a workaround until more complete variant support arrives.

Footnotes

1. I don't know where to_string() is coming from but I assume it operates on a &[u16]. ↩

[kennykerr]

// in a fn from_variant(vt: &VARIANT)
let variant_value = match variant_type {
VT_BSTR => {
let bstr_ptr: &BSTR = unsafe { &vt.Anonymous.Anonymous.Anonymous.bstrVal };

    Variant::String(bstr_ptr.try_into()?)
}
...

}

Taking a step back, is there an option to use VARIANT directly as it provides From/Into to convert between BSTR and other types? That should be a lot simpler.

[ohadravid]

.. is there an option to use VARIANT directly

Yes you are right @kennykerr, I still need as_raw for other stuff but I can use the VARIANT directly for the BSTR case which simplify this 🥳

Thanks!