Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent allocation failure handling #3209

Merged
merged 1 commit into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ jobs:
run: cargo clippy -p test_array
- name: Clippy test_bcrypt
run: cargo clippy -p test_bcrypt
- name: Clippy test_bstr
run: cargo clippy -p test_bstr
- name: Clippy test_calling_convention
run: cargo clippy -p test_calling_convention
- name: Clippy test_cfg_generic
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ jobs:
run: cargo test -p test_array --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_bcrypt
run: cargo test -p test_bcrypt --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_bstr
run: cargo test -p test_bstr --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_calling_convention
run: cargo test -p test_calling_convention --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_cfg_generic
Expand Down Expand Up @@ -258,10 +256,10 @@ jobs:
run: cargo test -p test_standalone --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_string_param
run: cargo test -p test_string_param --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Clean
run: cargo clean
- name: Test test_strings
run: cargo test -p test_strings --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Clean
run: cargo clean
- name: Test test_structs
run: cargo test -p test_structs --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_sys
Expand Down
1 change: 0 additions & 1 deletion crates/libs/core/src/imp/com_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ pub const CO_E_NOTINITIALIZED: windows_core::HRESULT = windows_core::HRESULT(0x8
pub const E_BOUNDS: windows_core::HRESULT = windows_core::HRESULT(0x8000000B_u32 as _);
pub const E_INVALIDARG: windows_core::HRESULT = windows_core::HRESULT(0x80070057_u32 as _);
pub const E_NOINTERFACE: windows_core::HRESULT = windows_core::HRESULT(0x80004002_u32 as _);
pub const E_OUTOFMEMORY: windows_core::HRESULT = windows_core::HRESULT(0x8007000E_u32 as _);
pub const E_POINTER: windows_core::HRESULT = windows_core::HRESULT(0x80004003_u32 as _);
windows_core::imp::define_interface!(
IAgileObject,
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/registry/src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl Key {
return Err(invalid_data());
}

let mut value = HStringBuilder::new(len / 2)?;
let mut value = HStringBuilder::new(len / 2);
unsafe { self.raw_get_bytes(name.as_raw(), value.as_bytes_mut())? };
value.trim_end();
Ok(value.into())
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/registry/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl TryFrom<Value> for HSTRING {
type Error = Error;
fn try_from(from: Value) -> Result<Self> {
match from.ty {
Type::String | Type::ExpandString => Ok(Self::from_wide(trim(from.data.as_wide()))?),
Type::String | Type::ExpandString => Ok(Self::from_wide(trim(from.data.as_wide()))),
_ => Err(invalid_data()),
}
}
Expand Down
5 changes: 0 additions & 5 deletions crates/libs/strings/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ targets = []
version = "0.52.6"
path = "../targets"

[dependencies.windows-result]
version = "0.2.0"
path = "../result"
default-features = false

[features]
default = ["std"]
std = []
8 changes: 3 additions & 5 deletions crates/libs/strings/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,14 @@ use windows_strings::*;
const A: PCSTR = s!("ansi");
const W: PCWSTR = w!("wide");

fn main() -> Result<()> {
fn main() {
let b = BSTR::from("bstr");
let h = HSTRING::from("hstring");

assert_eq!(b, "bstr");
assert_eq!(h, "hstring");

assert_eq!(unsafe { A.to_string()? }, "ansi");
assert_eq!(unsafe { W.to_string()? }, "wide");

Ok(())
assert_eq!(unsafe { A.to_string().unwrap() }, "ansi");
assert_eq!(unsafe { W.to_string().unwrap() }, "wide");
}
```
2 changes: 0 additions & 2 deletions crates/libs/strings/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ windows_targets::link!("oleaut32.dll" "system" fn SysFreeString(bstrstring : BST
windows_targets::link!("oleaut32.dll" "system" fn SysStringLen(pbstr : BSTR) -> u32);
pub type BOOL = i32;
pub type BSTR = *const u16;
pub const E_OUTOFMEMORY: HRESULT = 0x8007000E_u32 as _;
pub type HANDLE = *mut core::ffi::c_void;
pub type HEAP_FLAGS = u32;
pub type HRESULT = i32;
pub type PCWSTR = *const u16;
16 changes: 8 additions & 8 deletions crates/libs/strings/src/bstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,23 @@ impl BSTR {
}

/// Create a `BSTR` from a slice of 16 bit characters (wchars).
pub fn from_wide(value: &[u16]) -> Result<Self> {
pub fn from_wide(value: &[u16]) -> Self {
if value.is_empty() {
return Ok(Self::new());
return Self::new();
}

let result = unsafe {
Self(bindings::SysAllocStringLen(
value.as_ptr(),
value.len().try_into()?,
value.len().try_into().unwrap(),
))
};

if result.is_empty() {
Err(Error::from_hresult(HRESULT(bindings::E_OUTOFMEMORY)))
} else {
Ok(result)
panic!("allocation failed");
}

result
}

/// # Safety
Expand All @@ -77,14 +77,14 @@ impl BSTR {

impl Clone for BSTR {
fn clone(&self) -> Self {
Self::from_wide(self.as_wide()).unwrap()
Self::from_wide(self.as_wide())
}
}

impl From<&str> for BSTR {
fn from(value: &str) -> Self {
let value: alloc::vec::Vec<u16> = value.encode_utf16().collect();
Self::from_wide(&value).unwrap()
Self::from_wide(&value)
}
}

Expand Down
15 changes: 7 additions & 8 deletions crates/libs/strings/src/hstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl HSTRING {
}

/// Create a `HSTRING` from a slice of 16 bit characters (wchars).
pub fn from_wide(value: &[u16]) -> Result<Self> {
pub fn from_wide(value: &[u16]) -> Self {
unsafe { Self::from_wide_iter(value.iter().copied(), value.len()) }
}

Expand All @@ -61,12 +61,12 @@ impl HSTRING {

/// # Safety
/// len must not be less than the number of items in the iterator.
unsafe fn from_wide_iter<I: Iterator<Item = u16>>(iter: I, len: usize) -> Result<Self> {
unsafe fn from_wide_iter<I: Iterator<Item = u16>>(iter: I, len: usize) -> Self {
if len == 0 {
return Ok(Self::new());
return Self::new();
}

let ptr = HStringHeader::alloc(len.try_into()?)?;
let ptr = HStringHeader::alloc(len.try_into().unwrap());

// Place each utf-16 character into the buffer and
// increase len as we go along.
Expand All @@ -79,7 +79,7 @@ impl HSTRING {

// Write a 0 byte to the end of the buffer.
(*ptr).data.offset((*ptr).len as isize).write(0);
Ok(Self(ptr))
Self(ptr)
}

fn as_header(&self) -> Option<&HStringHeader> {
Expand All @@ -96,7 +96,7 @@ impl Default for HSTRING {
impl Clone for HSTRING {
fn clone(&self) -> Self {
if let Some(header) = self.as_header() {
Self(header.duplicate().unwrap())
Self(header.duplicate())
} else {
Self::new()
}
Expand Down Expand Up @@ -138,7 +138,7 @@ impl core::fmt::Debug for HSTRING {

impl From<&str> for HSTRING {
fn from(value: &str) -> Self {
unsafe { Self::from_wide_iter(value.encode_utf16(), value.len()).unwrap() }
unsafe { Self::from_wide_iter(value.encode_utf16(), value.len()) }
}
}

Expand Down Expand Up @@ -169,7 +169,6 @@ impl From<&std::ffi::OsStr> for HSTRING {
std::os::windows::ffi::OsStrExt::encode_wide(value),
value.len(),
)
.unwrap()
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/libs/strings/src/hstring_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ pub struct HStringBuilder(*mut HStringHeader);

impl HStringBuilder {
/// Creates a preallocated `HSTRING` value.
pub fn new(len: usize) -> Result<Self> {
let header = HStringHeader::alloc(len.try_into()?)?;
pub fn new(len: usize) -> Self {
let header = HStringHeader::alloc(len.try_into().unwrap());

if len > 0 {
unsafe { core::ptr::write_bytes((*header).data, 0, len) };
}

Ok(Self(header))
Self(header)
}

/// Shortens the string by removing any trailing 0 characters.
Expand Down
16 changes: 8 additions & 8 deletions crates/libs/strings/src/hstring_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ pub struct HStringHeader {
}

impl HStringHeader {
pub fn alloc(len: u32) -> Result<*mut Self> {
pub fn alloc(len: u32) -> *mut Self {
if len == 0 {
return Ok(core::ptr::null_mut());
return core::ptr::null_mut();
}

// Allocate enough space for header and two bytes per character.
Expand All @@ -27,7 +27,7 @@ impl HStringHeader {
unsafe { bindings::HeapAlloc(bindings::GetProcessHeap(), 0, bytes) } as *mut Self;

if header.is_null() {
return Err(Error::from_hresult(HRESULT(bindings::E_OUTOFMEMORY)));
panic!("allocation failed");
}

unsafe {
Expand All @@ -38,7 +38,7 @@ impl HStringHeader {
(*header).data = &mut (*header).buffer_start;
}

Ok(header)
header
}

pub unsafe fn free(header: *mut Self) {
Expand All @@ -49,20 +49,20 @@ impl HStringHeader {
bindings::HeapFree(bindings::GetProcessHeap(), 0, header as *mut _);
}

pub fn duplicate(&self) -> Result<*mut Self> {
pub fn duplicate(&self) -> *mut Self {
if self.flags & HSTRING_REFERENCE_FLAG == 0 {
// If this is not a "fast pass" string then simply increment the reference count.
self.count.add_ref();
Ok(self as *const Self as *mut Self)
self as *const Self as *mut Self
} else {
// Otherwise, allocate a new string and copy the value into the new string.
let copy = Self::alloc(self.len)?;
let copy = Self::alloc(self.len);
// SAFETY: since we are duplicating the string it is safe to copy all data from self to the initialized `copy`.
// We copy `len + 1` characters since `len` does not account for the terminating null character.
unsafe {
core::ptr::copy_nonoverlapping(self.data, (*copy).data, self.len as usize + 1);
}
Ok(copy)
copy
}
}
}
3 changes: 0 additions & 3 deletions crates/libs/strings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@
extern crate alloc;
use alloc::string::String;

pub use windows_result::Result;
use windows_result::*;

mod bstr;
pub use bstr::*;

Expand Down
2 changes: 1 addition & 1 deletion crates/libs/strings/src/pcwstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl PCWSTR {
/// # Safety
///
/// See the safety information for `PCWSTR::as_wide`.
pub unsafe fn to_hstring(&self) -> Result<HSTRING> {
pub unsafe fn to_hstring(&self) -> HSTRING {
HSTRING::from_wide(self.as_wide())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/libs/strings/src/pwstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl PWSTR {
/// # Safety
///
/// See the safety information for `PWSTR::as_wide`.
pub unsafe fn to_hstring(&self) -> Result<HSTRING> {
pub unsafe fn to_hstring(&self) -> HSTRING {
HSTRING::from_wide(self.as_wide())
}

Expand Down
15 changes: 0 additions & 15 deletions crates/tests/bstr/Cargo.toml

This file was deleted.

1 change: 0 additions & 1 deletion crates/tests/bstr/src/lib.rs

This file was deleted.

62 changes: 0 additions & 62 deletions crates/tests/bstr/tests/bstr.rs

This file was deleted.

Loading