Skip to content

Commit

Permalink
generator: Wrap _as_c_str() getter for possibly-pointers in Option (
Browse files Browse the repository at this point in the history 
#860)

While this function is already marked `unsafe` to represent cases where
an invalid pointer might be dereferenced, it should at least be obvious
to the caller that there is a real chance for `NULL` pointers in these
`CStr` getters, which will now be returned as `None`.  This function
won't be used in `Debug` now as the dereference operation is still
`unsafe`.

The `_as_c_str()` getters for static arrays is left untouched, as the
data is read directly from the known-valid struct here.
  • Loading branch information
@MarijnS95
MarijnS95 authored Jan 11, 2024
1 parent e992225 commit 92084df
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 32 deletions.
112 changes: 84 additions & 28 deletions ash/src/vk/definitions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1062,8 +1062,12 @@ impl<'a> ApplicationInfo<'a> {
self
}
#[inline]
pub unsafe fn application_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_application_name)
pub unsafe fn application_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_application_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_application_name))
}
}
#[inline]
pub fn application_version(mut self, application_version: u32) -> Self {
Expand All @@ -1076,8 +1080,12 @@ impl<'a> ApplicationInfo<'a> {
self
}
#[inline]
pub unsafe fn engine_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_engine_name)
pub unsafe fn engine_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_engine_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_engine_name))
}
}
#[inline]
pub fn engine_version(mut self, engine_version: u32) -> Self {
Expand Down Expand Up @@ -3755,8 +3763,12 @@ impl<'a> PipelineShaderStageCreateInfo<'a> {
self
}
#[inline]
pub unsafe fn name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_name)
pub unsafe fn name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_name))
}
}
#[inline]
pub fn specialization_info(mut self, specialization_info: &'a SpecializationInfo<'a>) -> Self {
Expand Down Expand Up @@ -7869,8 +7881,12 @@ impl<'a> DisplayPropertiesKHR<'a> {
self
}
#[inline]
pub unsafe fn display_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.display_name)
pub unsafe fn display_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.display_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.display_name))
}
}
#[inline]
pub fn physical_dimensions(mut self, physical_dimensions: Extent2D) -> Self {
Expand Down Expand Up @@ -9172,8 +9188,12 @@ impl<'a> DebugMarkerObjectNameInfoEXT<'a> {
self
}
#[inline]
pub unsafe fn object_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_object_name)
pub unsafe fn object_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_object_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_object_name))
}
}
}
#[repr(C)]
Expand Down Expand Up @@ -9266,8 +9286,12 @@ impl<'a> DebugMarkerMarkerInfoEXT<'a> {
self
}
#[inline]
pub unsafe fn marker_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_marker_name)
pub unsafe fn marker_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_marker_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_marker_name))
}
}
#[inline]
pub fn color(mut self, color: [f32; 4]) -> Self {
Expand Down Expand Up @@ -18726,8 +18750,12 @@ impl<'a> DebugUtilsObjectNameInfoEXT<'a> {
self
}
#[inline]
pub unsafe fn object_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_object_name)
pub unsafe fn object_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_object_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_object_name))
}
}
}
#[repr(C)]
Expand Down Expand Up @@ -18816,8 +18844,12 @@ impl<'a> DebugUtilsLabelEXT<'a> {
self
}
#[inline]
pub unsafe fn label_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_label_name)
pub unsafe fn label_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_label_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_label_name))
}
}
#[inline]
pub fn color(mut self, color: [f32; 4]) -> Self {
Expand Down Expand Up @@ -18961,8 +18993,12 @@ impl<'a> DebugUtilsMessengerCallbackDataEXT<'a> {
self
}
#[inline]
pub unsafe fn message_id_name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_message_id_name)
pub unsafe fn message_id_name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_message_id_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_message_id_name))
}
}
#[inline]
pub fn message_id_number(mut self, message_id_number: i32) -> Self {
Expand All @@ -18975,8 +19011,12 @@ impl<'a> DebugUtilsMessengerCallbackDataEXT<'a> {
self
}
#[inline]
pub unsafe fn message_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_message)
pub unsafe fn message_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_message.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_message))
}
}
#[inline]
pub fn queue_labels(mut self, queue_labels: &'a [DebugUtilsLabelEXT<'a>]) -> Self {
Expand Down Expand Up @@ -42334,8 +42374,12 @@ impl<'a> CuFunctionCreateInfoNVX<'a> {
self
}
#[inline]
pub unsafe fn name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_name)
pub unsafe fn name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_name))
}
}
}
#[repr(C)]
Expand Down Expand Up @@ -44790,8 +44834,12 @@ impl<'a> CudaFunctionCreateInfoNV<'a> {
self
}
#[inline]
pub unsafe fn name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_name)
pub unsafe fn name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_name))
}
}
}
#[repr(C)]
Expand Down Expand Up @@ -51450,8 +51498,12 @@ impl<'a> ShaderCreateInfoEXT<'a> {
self
}
#[inline]
pub unsafe fn name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_name)
pub unsafe fn name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_name))
}
}
#[inline]
pub fn set_layouts(mut self, set_layouts: &'a [DescriptorSetLayout]) -> Self {
Expand Down Expand Up @@ -52262,8 +52314,12 @@ impl<'a> PipelineShaderStageNodeCreateInfoAMDX<'a> {
self
}
#[inline]
pub unsafe fn name_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.p_name)
pub unsafe fn name_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.p_name.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.p_name))
}
}
#[inline]
pub fn index(mut self, index: u32) -> Self {
Expand Down
15 changes: 11 additions & 4 deletions generator/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,8 @@ impl FieldExt for vkxml::Field {

fn is_pointer_to_static_sized_array(&self) -> bool {
matches!(self.array, Some(vkxml::ArrayType::Dynamic))
// TODO: This should not be hardcoded to one field name, but recognize this pattern somehow
// (by checking if the len field is an expression consisting of purely constants)
&& self.name.as_deref() == Some("pVersionData")
}
}
Expand Down Expand Up @@ -2009,8 +2011,12 @@ fn derive_getters_and_setters(
}
#deprecated
#[inline]
pub unsafe fn #param_ident_as_c_str(&self) -> &core::ffi::CStr {
core::ffi::CStr::from_ptr(self.#param_ident)
pub unsafe fn #param_ident_as_c_str(&self) -> Option<&core::ffi::CStr> {
if self.#param_ident.is_null() {
None
} else {
Some(core::ffi::CStr::from_ptr(self.#param_ident))
}
}
});
} else if is_static_array(field) {
Expand Down Expand Up @@ -2047,8 +2053,9 @@ fn derive_getters_and_setters(

let mut slice_param_ty_tokens = field.safe_type_tokens(quote!('a), type_lifetime.clone(), None);

// vkxml wrongly annotates static arrays with a len= field is dynamic. These fields have a static length,
// and a runtime field to describe the actual number of valid items in this static array.
// vkxml considers static arrays with len= to be Dynamic (which they are to some
// extent). These fields have a static upper-bound length, and a runtime field to
// describe the actual number of valid items in this static array.
if is_static_array(field) {
let array_size_ident = format_ident!("{}", array_size.to_snake_case());
let param_ident_short_as_slice = format_ident!("{}_as_slice", param_ident_short);
Expand Down

0 comments on commit 92084df

Please sign in to comment.