Skip to content

BytesCursor abstraction in FuseDevWriter - #221

Open
stepancheg wants to merge 2 commits into
cloud-hypervisor:masterfrom
stepancheg:bytes-cursor
Open

BytesCursor abstraction in FuseDevWriter#221
stepancheg wants to merge 2 commits into
cloud-hypervisor:masterfrom
stepancheg:bytes-cursor

Conversation

@stepancheg

Copy link
Copy Markdown

FuseDevWriter.buf field is ManuallyDrop<Vec<u8>>, but actually it is a pair of &mut [u8] space and position.

Express this logic more explicitly with

pub(crate) struct BytesCursor<'a> {
    slice: &'a mut [u8],
    position: usize,
}

Because:

  • less unsafe code
  • more operations are checked
  • with this change it is clear that there's no UB (for example, related to aliasing &mut pointers). It may be not an issue, but with this change is much more clear

@bergwolf

Copy link
Copy Markdown
Contributor

somehow CI is not running. Let me close and reopen to trigger it.

@bergwolf bergwolf closed this Feb 25, 2026
@bergwolf bergwolf reopened this Feb 25, 2026
fn test_extend_from_slice() {
let mut array = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
let mut b = BytesCursor::new(&mut array, 3);
b.extend_from_slice(&[b'a', b'b']);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: clippy (byte_char_slices) flags this and CI runs with -Dwarnings:

    b.extend_from_slice(b"ab");

Ok(cnt)
} else {
Self::do_write(self.fd, &self.buf[..cnt])
Self::do_write(self.fd, &self.buf.slice_mut()[..cnt])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the sibling call site in write_from uses self.buf.written()[..cnt]. Both are equivalent here (position was 0 before the write), but written()[..cnt] expresses the intent more clearly — could use it here too for consistency?

  Self::do_write(self.fd, &self.buf.written()[..cnt])

/// device write operation.
/// 3. Concurrency, caller should not write to the writer concurrently.
#[derive(Debug, PartialEq, Eq)]
#[derive(Debug)]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads-up: FuseDevWriter is a public type, so removing PartialEq/Eq is technically an API-breaking change. I checked nothing in this repo relies on it, but it's probably worth a one-liner in the changelog.
Or we may keep it as is.

@@ -0,0 +1,110 @@
use crate::transport::Error;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please help to add copyright header?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants