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

Consider improving accessibility of working with flexible array members in the windows crate #3172

Closed
ChayimFriedman2 opened this issue Jul 22, 2024 · 2 comments
Labels
question Further information is requested

Comments

@ChayimFriedman2
Copy link

ChayimFriedman2 commented Jul 22, 2024

Suggestion

Many Windows APIs return flexible array members. Working with them in Rust is terrible. You must only use raw pointers, as references will invalidate access to the undeclared part of the array in Stacked Borrows (rust-lang/unsafe-code-guidelines#256), and it's easy to make mistakes and cause UB.

We can generate a simple wrapper type per FAM that can simplify working with them a lot. Something along the lines of:

// Assume the following simple struct:
#[repr(C)]
pub struct Fam {
    len: usize,
    other: u32,
    array: [i32; 1],
}

// We generate the following code:
#[repr(transparent)]
pub struct FamPtr(*mut Fam);

impl FamPtr {
    pub fn as_raw(&self) -> *mut Fam { self.0 }
    pub unsafe fn from_raw(raw: *mut Fam) -> Self { Self(raw) }
    
    pub fn len(&self) -> &usize {
        unsafe { &(*self.0).len }
    }
    // No `len_mut()`, that would be unsound.
    pub fn other(&self) -> &u32 {
        unsafe { &(*self.0).other }
    }
    pub fn other_mut(&mut self) -> &mut u32 {
        unsafe { &mut (*self.0).other }
    }
    
    pub fn array(&self) -> &[i32] {
        unsafe {
            core::slice::from_raw_parts(
                core::ptr::addr_of!((*self.0).array).cast::<i32>(),
                (*self.0).len,
            )
        }
    }
    pub fn array_mut(&mut self) -> &[i32] {
        unsafe {
            core::slice::from_raw_parts_mut(
                core::ptr::addr_of_mut!((*self.0).array).cast::<i32>(),
                (*self.0).len,
            )
        }
    }
}

There can be additional extensions (for example, supporting taking mutable references to many array members at the same time, safely allocating FAMs on the stack or the heap, or freeing them automatically) but even that alone is way better than the status quo.

Every function taking a pointer to a flexible struct will gain the ability to call it with its wrapper instead (probably with a trait).

I don't know if win32metadata marks flexible array members and their length field, so I don't know if this is possible, but if it is it can be a major improvement.

@ChayimFriedman2 ChayimFriedman2 added the enhancement New feature or request label Jul 22, 2024
@riverar
Copy link
Collaborator

riverar commented Jul 22, 2024

Just a data point: In upstream metadata, we do try to find/label flexible array members. We don't currently associate their length with any fields, however.

@kennykerr kennykerr added question Further information is requested and removed enhancement New feature or request labels Aug 1, 2024
@kennykerr
Copy link
Collaborator

While not a bad idea, this is hard to generalize for code generation. I can see this being helpful for hand-authored wrapper crates though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants