Skip to content

Commit

Permalink
hal: Introduce cstr_from_bytes_until_nul helper function.
Browse files Browse the repository at this point in the history
Remove a handful of `unsafe` blocks from the Vulkan back end.
  • Loading branch information
jimblandy committed Oct 7, 2022
1 parent 130ba89 commit 16f2702
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 32 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ Bottom level categories:

#### Vulkan

- Don't use a pointer to a local copy of a `PhysicalDeviceDriverProperties` struct after it has gone out of scope. In fact, don't make a local copy at all. By @jimblandy in [#3076](https://github.com/gfx-rs/wgpu/pull/3076).
- Don't use a pointer to a local copy of a `PhysicalDeviceDriverProperties` struct after it has gone out of scope. In fact, don't make a local copy at all. Introduce a helper function for building `CStr`s from C character arrays, and remove some `unsafe` blocks. By @jimblandy in [#3076](https://github.com/gfx-rs/wgpu/pull/3076).

## wgpu-0.14.0 (2022-10-05)

Expand Down
21 changes: 21 additions & 0 deletions wgpu-hal/src/auxil/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,24 @@ impl crate::TextureCopy {
self.size = self.size.min(&max_src_size).min(&max_dst_size);
}
}

/// Construct a `CStr` from a byte slice, up to the first zero byte.
///
/// Return a `CStr` extending from the start of `bytes` up to and
/// including the first zero byte. If there is no zero byte in
/// `bytes`, return `None`.
///
/// This can be removed when `CStr::from_bytes_until_nul` is stabilized.
/// ([#95027](https://github.com/rust-lang/rust/issues/95027))
#[allow(dead_code)]
pub(crate) fn cstr_from_bytes_until_nul(bytes: &[std::ffi::c_char]) -> Option<&std::ffi::CStr> {
if bytes.contains(&0) {
// Safety for `CStr::from_ptr`:
// - We've ensured that the slice does contain a null terminator.
// - The range is valid to read, because the slice covers it.
// - The memory won't be changed, because the slice borrows it.
unsafe { Some(std::ffi::CStr::from_ptr(bytes.as_ptr())) }
} else {
None
}
}
42 changes: 22 additions & 20 deletions wgpu-hal/src/vulkan/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,9 +538,10 @@ impl PhysicalDeviceCapabilities {
}

pub fn supports_extension(&self, extension: &CStr) -> bool {
use crate::auxil::cstr_from_bytes_until_nul;
self.supported_extensions
.iter()
.any(|ep| unsafe { CStr::from_ptr(ep.extension_name.as_ptr()) } == extension)
.any(|ep| cstr_from_bytes_until_nul(&ep.extension_name) == Some(extension))
}

/// Map `requested_features` to the list of Vulkan extension strings required to create the logical device.
Expand Down Expand Up @@ -885,14 +886,15 @@ impl super::Instance {
&self,
phd: vk::PhysicalDevice,
) -> Option<crate::ExposedAdapter<super::Api>> {
use crate::auxil::cstr_from_bytes_until_nul;
use crate::auxil::db;

let (phd_capabilities, phd_features) = self.shared.inspect(phd);

let info = wgt::AdapterInfo {
name: unsafe {
CStr::from_ptr(phd_capabilities.properties.device_name.as_ptr())
.to_str()
name: {
cstr_from_bytes_until_nul(&phd_capabilities.properties.device_name)
.and_then(|info| info.to_str().ok())
.unwrap_or("?")
.to_owned()
},
Expand All @@ -906,23 +908,23 @@ impl super::Instance {
ash::vk::PhysicalDeviceType::CPU => wgt::DeviceType::Cpu,
_ => wgt::DeviceType::Other,
},
driver: unsafe {
let driver_name = if let Some(ref driver) = phd_capabilities.driver {
CStr::from_ptr(driver.driver_name.as_ptr()).to_str().ok()
} else {
None
};

driver_name.unwrap_or("?").to_owned()
driver: {
phd_capabilities
.driver
.as_ref()
.and_then(|driver| cstr_from_bytes_until_nul(&driver.driver_name))
.and_then(|name| name.to_str().ok())
.unwrap_or("?")
.to_owned()
},
driver_info: unsafe {
let driver_info = if let Some(ref driver) = phd_capabilities.driver {
CStr::from_ptr(driver.driver_info.as_ptr()).to_str().ok()
} else {
None
};

driver_info.unwrap_or("?").to_owned()
driver_info: {
phd_capabilities
.driver
.as_ref()
.and_then(|driver| cstr_from_bytes_until_nul(&driver.driver_info))
.and_then(|name| name.to_str().ok())
.unwrap_or("?")
.to_owned()
},
backend: wgt::Backend::Vulkan,
};
Expand Down
22 changes: 11 additions & 11 deletions wgpu-hal/src/vulkan/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,9 @@ impl super::Instance {

// Only keep available extensions.
extensions.retain(|&ext| {
if instance_extensions
.iter()
.any(|inst_ext| unsafe { CStr::from_ptr(inst_ext.extension_name.as_ptr()) == ext })
{
if instance_extensions.iter().any(|inst_ext| {
crate::auxil::cstr_from_bytes_until_nul(&inst_ext.extension_name) == Some(ext)
}) {
true
} else {
log::info!("Unable to find extension: {}", ext.to_string_lossy());
Expand Down Expand Up @@ -483,6 +482,8 @@ impl Drop for super::InstanceShared {

impl crate::Instance<super::Api> for super::Instance {
unsafe fn init(desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
use crate::auxil::cstr_from_bytes_until_nul;

let entry = match ash::Entry::load() {
Ok(entry) => entry,
Err(err) => {
Expand Down Expand Up @@ -531,9 +532,9 @@ impl crate::Instance<super::Api> for super::Instance {
})?;

let nv_optimus_layer = CStr::from_bytes_with_nul(b"VK_LAYER_NV_optimus\0").unwrap();
let has_nv_optimus = instance_layers
.iter()
.any(|inst_layer| CStr::from_ptr(inst_layer.layer_name.as_ptr()) == nv_optimus_layer);
let has_nv_optimus = instance_layers.iter().any(|inst_layer| {
cstr_from_bytes_until_nul(&inst_layer.layer_name) == Some(nv_optimus_layer)
});

// Check requested layers against the available layers
let layers = {
Expand All @@ -544,10 +545,9 @@ impl crate::Instance<super::Api> for super::Instance {

// Only keep available layers.
layers.retain(|&layer| {
if instance_layers
.iter()
.any(|inst_layer| CStr::from_ptr(inst_layer.layer_name.as_ptr()) == layer)
{
if instance_layers.iter().any(|inst_layer| {
cstr_from_bytes_until_nul(&inst_layer.layer_name) == Some(layer)
}) {
true
} else {
log::warn!("Unable to find layer: {}", layer.to_string_lossy());
Expand Down

0 comments on commit 16f2702

Please sign in to comment.