search

Ralith / openxrs

OpenXR bindings for Rust
Apache License 2.0
282 stars 59 forks source link

Fix UB in `Instance::supports_<prop>()` #170

Closed zmerp closed 2 months ago

zmerp commented 2 months ago

Closes #169

This solution works but it would be even more concise if the structs sys::System<Prop> would implement Default. The Default impl should already set the correct ty, and maybe the out() methods should base off the default value and return T instead of MaybeUninit<T>. As per review, I switched to checking if the extension is present instead.

Ralith commented 2 months ago

Does the spec guarantee defined behavior when passing an extension struct from a disabled extension at all? I think it would be safer to check if the extension is enabled and bail out without making any XR calls at all if not.

zmerp commented 2 months ago

Ok. Should I revert back to using T::out()?

Ralith commented 2 months ago

That would be more consistent, yeah.

Ralith commented 2 months ago

It looks like the answer to my question above is actually "yes":

A runtime must ignore all unrecognized structures in a next chain, including those associated with an extension that has not been enabled.

So either approach seems fine. That said, the current form is concise and might make user error a bit more obvious, so I'm happy with it.

zmerp commented 2 months ago

I'm sorry I made a mistake. I opened another PR to check the correct extension