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

add ReadBuf::buf* accessor functions for getting the original buffer #65

Closed
programmerjake opened this issue Jul 8, 2022 · 3 comments
Closed
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@programmerjake
Copy link
Member

programmerjake commented Jul 8, 2022

Proposal

Problem statement

There is no way to retrieve the full, originally passed in buffer from ReadBuf

Motivation, use-cases

extern "C" {
    fn expand_in_place(buf: *mut u8, buf_len: usize, buf_used: usize) -> usize;
}
const SZ: usize = 64;
let mut buf = MaybeUninit::<[_; SZ]>::uninit().assume_init();
let mut read_buf = ReadBuf::uninit(&mut buf);
reader.read_buf(&mut read_buf)?;
// can't just use original buffer reference because reader.read_buf might swap it out, causing uninitialized data to be accessed
unsafe {
    let used = read_buf.filled_len();
    let len = read_buf.capacity();
    let ptr = read_buf.buf_mut() as *mut _; // new API here, nothing existing returns the full buffer, just a slice
    let used = expand_in_place(ptr as *mut u8, len, used);
    println!("{}", slice::from_raw_parts_mut(ptr, used));
}

Solution sketches

impl<'a> ReadBuf<'a> {
    /// Returns the buffer
    #[inline]
    pub fn buf(&self) -> &[MaybeUninit<u8>] {
        &*self.buf
    }

    /// Returns the buffer
    ///
    /// # Safety
    /// You must not write unitialized bytes to positions less than `self.initialized_len()`
    #[inline]
    pub unsafe fn buf_mut(&mut self) -> &mut [MaybeUninit<u8>] {
        self.buf
    }

    /// Returns the buffer
    #[inline]
    pub fn into_buf(self) -> &'a mut [MaybeUninit<u8>] {
        self.buf
    }
}

Links and related work

rust-lang/rust#98962
rust-lang/rust#78485

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@programmerjake programmerjake added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Jul 8, 2022
@the8472
Copy link
Member

the8472 commented Jul 8, 2022

// can't just use original buffer reference because reader.read_buf might swap it out, causing uninitialized data to be accessed

rust-lang/rust#97015 already aims to address that.

@josephlr
Copy link

josephlr commented Jul 9, 2022

Could we also have some way to extract the initialized bytes from the ReadBuf once it is done being used.

After using this API a little bit, I encountered a pattern that I think would be fairly common:

  • Start with some large uninitialized buffer (with lifetime &'a mut [MaybeUninit<u8>])
  • Create a ReadBuf<'a>
  • Pass this ReadBuf to various Read methods that gradually fill most/all of the buffer
  • Now I want to return a &'a [u8] or &'a mut [u8] pointing to these filled and initialized bytes.

Without such an API I have to manually write unsafe code to convert my original &'a mut [MaybeUninit<u8>] to the appropriate initialized buffer type (with the correct length), which seems less than ideal.

Could we include a method that looks like:

#[inline]
pub fn into_filled_buf(self) -> &'a mut [u8] {
    unsafe { MaybeUninit::slice_assume_init_mut(&mut self.buf[0..self.filled]) }
}

@joshtriplett
Copy link
Member

We discussed this in today's libs meetup. As far as we can tell, the new BorrowedCursor API no longer allows swapping out the buffer, so the comment in the original motivation no longer holds. It should be possible to just use the original buffer.

If that's not the case, and there's still a use case for this. please follow up with an explanation of that use case.

@dtolnay dtolnay closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

5 participants