Switch to CStr::from_bytes_until_nul on sized char arrays in structs

Certain structs contain sized character arrays that are converted to `CStr` for debugging using unsafe `CStr::from_ptr(...as_ptr())`. There is no need to round-trip to a pointer and possibly read out of bounds if the NULL character index (string length) is instead scanned for manually on the slice or via the newly stabilized `CStr::from_bytes_until_nul()` since Rust 1.69 (which panics if no NULL char is found before the end of the slice). Unfortunately `unsafe` is still needed to cast the array from a `c_char` (`i8` on most platforms) to `u8`, which is what `from_bytes_until_nul()` accepts.
ash-rs · Oct 11, 2023 · 9229dc7 · 9229dc7
1 parent 3f5b96b
commit 9229dc7
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 97 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -11,19 +11,19 @@ jobs:
       - run: cargo check --workspace --all-targets --all-features
 
   check_msrv:
-    name: Check ash MSRV (1.60.0)
+    name: Check ash MSRV (1.69.0)
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v1
-      - uses: dtolnay/rust-toolchain@1.60.0
+      - uses: dtolnay/rust-toolchain@1.69.0
       - run: cargo check -p ash -p ash-rewrite --all-features
 
   check_ash_window_msrv:
-    name: Check ash-window MSRV (1.64.0)
+    name: Check ash-window MSRV (1.69.0)
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v1
-      - uses: dtolnay/rust-toolchain@1.64.0
+      - uses: dtolnay/rust-toolchain@1.69.0
       - run: cargo check -p ash-window -p examples --all-features
 
   # TODO: add a similar job for the rewrite once that generates code

diff --git a/Changelog.md b/Changelog.md
@@ -30,7 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Replaced builders with lifetimes/setters directly on Vulkan structs (#602)
 - Inlined struct setters (#602)
-- Bumped MSRV from 1.59 to 1.60 (#709)
+- Bumped MSRV from 1.59 to 1.69 (#709, #746)
 - Replaced `const fn name()` with associated `NAME` constants (#715)
 - Generic builders now automatically set `objecttype` to `<T as Handle>::ObjectType` (#724)
 - `get_calibrated_timestamps()` now returns a single value for `max_deviation` (#738)

diff --git a/README.md b/README.md
@@ -8,7 +8,7 @@ A very lightweight wrapper around Vulkan
 [![LICENSE](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE-MIT)
 [![LICENSE](https://img.shields.io/badge/license-Apache--2.0-blue.svg)](LICENSE-APACHE)
 [![Join the chat at https://gitter.im/MaikKlein/ash](https://badges.gitter.im/MaikKlein/ash.svg)](https://gitter.im/MaikKlein/ash?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
-[![MSRV](https://img.shields.io/badge/rustc-1.60.0+-ab6000.svg)](https://blog.rust-lang.org/2022/04/07/Rust-1.60.0.html)
+[![MSRV](https://img.shields.io/badge/rustc-1.69.0+-ab6000.svg)](https://blog.rust-lang.org/2023/04/20/Rust-1.69.0.html)
 
 ## Overview
 

diff --git a/ash-rewrite/Cargo.toml b/ash-rewrite/Cargo.toml
@@ -16,7 +16,7 @@ categories = [
 documentation = "https://docs.rs/ash"
 edition = "2021"
 # TODO: reevaluate, then update in ci.yml
-rust-version = "1.60.0"
+rust-version = "1.69.0"
 
 [dependencies]
 libloading = { version = "0.7", optional = true }

diff --git a/ash-window/Cargo.toml b/ash-window/Cargo.toml
@@ -12,7 +12,7 @@ categories = ["game-engines", "graphics"]
 exclude = [".github/*"]
 workspace = ".."
 edition = "2021"
-rust-version = "1.64.0"
+rust-version = "1.69.0"
 
 [dependencies]
 ash = { path = "../ash", version = "0.37", default-features = false }

diff --git a/ash-window/Changelog.md b/ash-window/Changelog.md
@@ -2,7 +2,7 @@
 
 ## [Unreleased] - ReleaseDate
 
-- Bumped MSRV from 1.59 to 1.64 for `winit 0.28` and `raw-window-handle 0.5.1`. (#709, #716)
+- Bumped MSRV from 1.59 to 1.69 for `winit 0.28` and `raw-window-handle 0.5.1`, and `CStr::from_bytes_until_nul`. (#709, #716, #746)
 
 ## [0.12.0] - 2022-09-23
 

diff --git a/ash-window/README.md b/ash-window/README.md
@@ -8,7 +8,7 @@ Interoperability between [`ash`](https://github.com/MaikKlein/ash) and [`raw-win
 [![LICENSE](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE-MIT)
 [![LICENSE](https://img.shields.io/badge/license-apache-blue.svg)](LICENSE-APACHE)
 [![Join the chat at https://gitter.im/MaikKlein/ash](https://badges.gitter.im/MaikKlein/ash.svg)](https://gitter.im/MaikKlein/ash?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
-[![MSRV](https://img.shields.io/badge/rustc-1.64.0+-ab6000.svg)](https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html)
+[![MSRV](https://img.shields.io/badge/rustc-1.69.0+-ab6000.svg)](https://blog.rust-lang.org/2023/04/20/Rust-1.69.0.html)
 
 ## Usage
 

diff --git a/ash/Cargo.toml b/ash/Cargo.toml
@@ -13,7 +13,7 @@ readme = "../README.md"
 keywords = ["vulkan", "graphic"]
 documentation = "https://docs.rs/ash"
 edition = "2021"
-rust-version = "1.60.0"
+rust-version = "1.69.0"
 
 [dependencies]
 libloading = { version = "0.7", optional = true }

diff --git a/ash/src/vk/definitions.rs b/ash/src/vk/definitions.rs
@@ -817,9 +817,7 @@ impl fmt::Debug for PhysicalDeviceProperties {
             .field("vendor_id", &self.vendor_id)
             .field("device_id", &self.device_id)
             .field("device_type", &self.device_type)
-            .field("device_name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.device_name.as_ptr())
-            })
+            .field("device_name", &wrap_cstr_slice_until_nul(&self.device_name))
             .field("pipeline_cache_uuid", &self.pipeline_cache_uuid)
             .field("limits", &self.limits)
             .field("sparse_properties", &self.sparse_properties)
@@ -900,9 +898,10 @@ pub struct ExtensionProperties {
 impl fmt::Debug for ExtensionProperties {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         fmt.debug_struct("ExtensionProperties")
-            .field("extension_name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.extension_name.as_ptr())
-            })
+            .field(
+                "extension_name",
+                &wrap_cstr_slice_until_nul(&self.extension_name),
+            )
             .field("spec_version", &self.spec_version)
             .finish()
     }
@@ -941,14 +940,10 @@ pub struct LayerProperties {
 impl fmt::Debug for LayerProperties {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         fmt.debug_struct("LayerProperties")
-            .field("layer_name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.layer_name.as_ptr())
-            })
+            .field("layer_name", &wrap_cstr_slice_until_nul(&self.layer_name))
             .field("spec_version", &self.spec_version)
             .field("implementation_version", &self.implementation_version)
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .finish()
     }
 }
@@ -10900,12 +10895,8 @@ impl fmt::Debug for PhysicalDeviceDriverProperties<'_> {
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
             .field("driver_id", &self.driver_id)
-            .field("driver_name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr())
-            })
-            .field("driver_info", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr())
-            })
+            .field("driver_name", &wrap_cstr_slice_until_nul(&self.driver_name))
+            .field("driver_info", &wrap_cstr_slice_until_nul(&self.driver_info))
             .field("conformance_version", &self.conformance_version)
             .finish()
     }
@@ -27050,15 +27041,9 @@ impl fmt::Debug for PerformanceCounterDescriptionKHR<'_> {
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
             .field("flags", &self.flags)
-            .field("name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.name.as_ptr())
-            })
-            .field("category", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.category.as_ptr())
-            })
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("name", &wrap_cstr_slice_until_nul(&self.name))
+            .field("category", &wrap_cstr_slice_until_nul(&self.category))
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .finish()
     }
 }
@@ -28137,12 +28122,8 @@ impl fmt::Debug for PipelineExecutablePropertiesKHR<'_> {
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
             .field("stages", &self.stages)
-            .field("name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.name.as_ptr())
-            })
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("name", &wrap_cstr_slice_until_nul(&self.name))
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .field("subgroup_size", &self.subgroup_size)
             .finish()
     }
@@ -28257,12 +28238,8 @@ impl fmt::Debug for PipelineExecutableStatisticKHR<'_> {
         fmt.debug_struct("PipelineExecutableStatisticKHR")
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
-            .field("name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.name.as_ptr())
-            })
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("name", &wrap_cstr_slice_until_nul(&self.name))
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .field("format", &self.format)
             .field("value", &"union")
             .finish()
@@ -28326,12 +28303,8 @@ impl fmt::Debug for PipelineExecutableInternalRepresentationKHR<'_> {
         fmt.debug_struct("PipelineExecutableInternalRepresentationKHR")
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
-            .field("name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.name.as_ptr())
-            })
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("name", &wrap_cstr_slice_until_nul(&self.name))
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .field("is_text", &self.is_text)
             .field("data_size", &self.data_size)
             .field("p_data", &self.p_data)
@@ -29834,12 +29807,8 @@ impl fmt::Debug for PhysicalDeviceVulkan12Properties<'_> {
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
             .field("driver_id", &self.driver_id)
-            .field("driver_name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr())
-            })
-            .field("driver_info", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr())
-            })
+            .field("driver_name", &wrap_cstr_slice_until_nul(&self.driver_name))
+            .field("driver_info", &wrap_cstr_slice_until_nul(&self.driver_info))
             .field("conformance_version", &self.conformance_version)
             .field(
                 "denorm_behavior_independence",
@@ -31190,19 +31159,11 @@ impl fmt::Debug for PhysicalDeviceToolProperties<'_> {
         fmt.debug_struct("PhysicalDeviceToolProperties")
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
-            .field("name", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.name.as_ptr())
-            })
-            .field("version", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.version.as_ptr())
-            })
+            .field("name", &wrap_cstr_slice_until_nul(&self.name))
+            .field("version", &wrap_cstr_slice_until_nul(&self.version))
             .field("purposes", &self.purposes)
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
-            .field("layer", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.layer.as_ptr())
-            })
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
+            .field("layer", &wrap_cstr_slice_until_nul(&self.layer))
             .finish()
     }
 }
@@ -45383,9 +45344,7 @@ impl fmt::Debug for RenderPassSubpassFeedbackInfoEXT {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         fmt.debug_struct("RenderPassSubpassFeedbackInfoEXT")
             .field("subpass_merge_status", &self.subpass_merge_status)
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .field("post_merge_index", &self.post_merge_index)
             .finish()
     }
@@ -48154,9 +48113,7 @@ pub struct DeviceFaultVendorInfoEXT {
 impl fmt::Debug for DeviceFaultVendorInfoEXT {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         fmt.debug_struct("DeviceFaultVendorInfoEXT")
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .field("vendor_fault_code", &self.vendor_fault_code)
             .field("vendor_fault_data", &self.vendor_fault_data)
             .finish()
@@ -48252,9 +48209,7 @@ impl fmt::Debug for DeviceFaultInfoEXT<'_> {
         fmt.debug_struct("DeviceFaultInfoEXT")
             .field("s_type", &self.s_type)
             .field("p_next", &self.p_next)
-            .field("description", &unsafe {
-                ::std::ffi::CStr::from_ptr(self.description.as_ptr())
-            })
+            .field("description", &wrap_cstr_slice_until_nul(&self.description))
             .field("p_address_infos", &self.p_address_infos)
             .field("p_vendor_infos", &self.p_vendor_infos)
             .field("p_vendor_binary_data", &self.p_vendor_binary_data)

diff --git a/ash/src/vk/prelude.rs b/ash/src/vk/prelude.rs
@@ -59,3 +59,13 @@ impl From<vk::Extent2D> for vk::Rect2D {
 pub unsafe trait TaggedStructure {
     const STRUCTURE_TYPE: vk::StructureType;
 }
+
+#[cfg(feature = "debug")]
+pub(crate) fn wrap_cstr_slice_until_nul(
+    bytes: &[::core::ffi::c_char],
+) -> Result<&::core::ffi::CStr, ::core::ffi::FromBytesUntilNulError> {
+    ::core::ffi::CStr::from_bytes_until_nul(
+        // SAFETY: The cast from c_char to u8 is ok because a c_char is always one byte.
+        unsafe { ::core::slice::from_raw_parts(bytes.as_ptr().cast(), bytes.len()) },
+    )
+}
diff --git a/generator/src/lib.rs b/generator/src/lib.rs
@@ -1788,25 +1788,15 @@ fn derive_debug(
         let param_ident = field.param_ident();
         let param_str = param_ident.to_string();
         let debug_value = if is_static_array(field) && field.basetype == "char" {
-            quote! {
-                &unsafe {
-                    ::std::ffi::CStr::from_ptr(self.#param_ident.as_ptr())
-                }
-            }
+            quote!(&wrap_cstr_slice_until_nul(&self.#param_ident))
         } else if param_str.contains("pfn") {
-            quote! {
-                &(self.#param_ident.map(|x| x as *const ()))
-            }
+            quote!(&(self.#param_ident.map(|x| x as *const ())))
         } else if union_types.contains(field.basetype.as_str()) {
             quote!(&"union")
         } else {
-            quote! {
-                &self.#param_ident
-            }
+            quote!(&self.#param_ident)
         };
-        quote! {
-            .field(#param_str, #debug_value)
-        }
+        quote!(.field(#param_str, #debug_value))
     });
     let name_str = name.to_string();
     let lifetime = has_lifetime.then(|| quote!(<'_>));