From ee5e651cf334de1891220dbd4dd9cbb8bc0b0afc Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Fri, 26 May 2023 10:46:03 +0200 Subject: [PATCH] entry: Mark all `extern "C"` function-pointer calls `unsafe` We don't mark any of the extern calls to Vulkan function-pointers safe except for a few `Entry` functions. Their safety contract was easy to validate, but now that we exposed a constructor function to let the user provide function pointers in the form of `Entry::from_parts_1_1()` it is no longer safe to assume that we are calling the function that adheres to the contract specified in the Vulkan reference. Because we don't rigorously do this validation and safe-marking anywhere else either, mark these function calls as `unsafe`. Related discussion chain: https://github.com/ash-rs/ash/pull/748#discussion_r1186794284 --- Changelog.md | 2 +- ash/src/entry.rs | 69 +++++++++++++++++++++--------------------------- 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/Changelog.md b/Changelog.md index 8ce05df1b..a95074f98 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `VK_EXT_shader_object` device extension (#732) - Added missing `Device::get_device_queue2()` wrapper (#736) - Exposed `FramebufferCreateInfo::attachment_count()` builder for `vk::FramebufferCreateFlags::IMAGELESS` (#747) -- Allow building `Entry`/`Instance`/`Device` from handle+fns (#748) +- Allow building `Entry`/`Instance`/`Device` from handle+fns (see their `from_parts_1_x()` associated functions) (#748) ### Changed diff --git a/ash/src/entry.rs b/ash/src/entry.rs index f3052e2c7..695d7b27f 100644 --- a/ash/src/entry.rs +++ b/ash/src/entry.rs @@ -184,7 +184,7 @@ impl Entry { /// # use ash::{Entry, vk}; /// # fn main() -> Result<(), Box> { /// let entry = Entry::linked(); - /// match entry.try_enumerate_instance_version()? { + /// match unsafe { entry.try_enumerate_instance_version() }? { /// // Vulkan 1.1+ /// Some(version) => { /// let major = vk::version_major(version); @@ -197,22 +197,19 @@ impl Entry { /// # Ok(()) } /// ``` #[inline] - pub fn try_enumerate_instance_version(&self) -> VkResult> { - unsafe { - let mut api_version = 0; - let enumerate_instance_version: Option = { - let name = CStr::from_bytes_with_nul_unchecked(b"vkEnumerateInstanceVersion\0"); - mem::transmute((self.static_fn.get_instance_proc_addr)( - vk::Instance::null(), - name.as_ptr(), - )) - }; - if let Some(enumerate_instance_version) = enumerate_instance_version { - (enumerate_instance_version)(&mut api_version) - .result_with_success(Some(api_version)) - } else { - Ok(None) - } + pub unsafe fn try_enumerate_instance_version(&self) -> VkResult> { + let mut api_version = 0; + let enumerate_instance_version: Option = { + let name = CStr::from_bytes_with_nul_unchecked(b"vkEnumerateInstanceVersion\0"); + mem::transmute((self.static_fn.get_instance_proc_addr)( + vk::Instance::null(), + name.as_ptr(), + )) + }; + if let Some(enumerate_instance_version) = enumerate_instance_version { + (enumerate_instance_version)(&mut api_version).result_with_success(Some(api_version)) + } else { + Ok(None) } } @@ -240,29 +237,25 @@ impl Entry { /// #[inline] - pub fn enumerate_instance_layer_properties(&self) -> VkResult> { - unsafe { - read_into_uninitialized_vector(|count, data| { - (self.entry_fn_1_0.enumerate_instance_layer_properties)(count, data) - }) - } + pub unsafe fn enumerate_instance_layer_properties(&self) -> VkResult> { + read_into_uninitialized_vector(|count, data| { + (self.entry_fn_1_0.enumerate_instance_layer_properties)(count, data) + }) } /// #[inline] - pub fn enumerate_instance_extension_properties( + pub unsafe fn enumerate_instance_extension_properties( &self, layer_name: Option<&CStr>, ) -> VkResult> { - unsafe { - read_into_uninitialized_vector(|count, data| { - (self.entry_fn_1_0.enumerate_instance_extension_properties)( - layer_name.map_or(ptr::null(), |str| str.as_ptr()), - count, - data, - ) - }) - } + read_into_uninitialized_vector(|count, data| { + (self.entry_fn_1_0.enumerate_instance_extension_properties)( + layer_name.map_or(ptr::null(), |str| str.as_ptr()), + count, + data, + ) + }) } /// @@ -288,12 +281,10 @@ impl Entry { /// /// Please use [`try_enumerate_instance_version()`][Self::try_enumerate_instance_version()] instead. #[inline] - pub fn enumerate_instance_version(&self) -> VkResult { - unsafe { - let mut api_version = 0; - (self.entry_fn_1_1.enumerate_instance_version)(&mut api_version) - .result_with_success(api_version) - } + pub unsafe fn enumerate_instance_version(&self) -> VkResult { + let mut api_version = 0; + (self.entry_fn_1_1.enumerate_instance_version)(&mut api_version) + .result_with_success(api_version) } }