Ralith/openxrs

get_str doesn't reliably strip null bytes

str4d opened this issue · 2 comments

str4d commented

In #46 I noted the following crash in the vulkan example:

     Running `C:\Users\me\Documents\dev\rust\sat\openxrs\target\debug\examples\vulkan.exe`
loaded OpenXR runtime: Oculus 1.52.0
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: NulError(38, [86, 75, 95, 75, 72, 82, 95, 103, 101, 116, 95, 112, 104, 121, 115, 105, 99, 97, 108, 95, 100, 101, 118, 105, 99, 101, 95, 112, 114, 111, 112, 101, 114, 116, 105, 101, 115, 50, 0])', openxr\examples\vulkan.rs:65:34

It turns out that this is because for both Instance::vulkan_instance_extensions and Instance::vulkan_device_extensions, get_arr is returning an array with two trailing null bytes, but get_str assumes that the string will only contain one, and simply truncates a single byte.

I can temporarily fix this in the example with trim_end_matches('\x00'), but we should either understand where the extra null byte is coming from (and remove it), or alter get_str to strip as many null bytes as exist.

we should either understand where the extra null byte is coming from (and remove it)

I think this is a bug in the Oculus OpenXR runtime. Neither SteamVR nor Monado emit multiple nulls. The spec says:

bufferCountOutput is a pointer to the count of characters written (including terminating \0)

which does not seem to permit this behavior to me.

alter get_str to strip as many null bytes as exist.

This seems like a reasonable work-around to me, though ideally we'd report the bug upstream and remove the hack once they fix their implementation. Though this function doesn't exist at all in XR_KHR_vulkan_enable2 so it may not be all that important in the long term.

I ran into this. Didn't see this issue but created PR #52 to handle it on my end.