Skip to content

Conversation

@asder8215
Copy link
Contributor

Out of curiosity, I was taking a look at the BufReader's peek() method and ended up checking out the code for Buffer's read_more() method. For reference, this is original code of Buffer's read_more() method.

    /// Read more bytes into the buffer without discarding any of its contents
    pub fn read_more(&mut self, mut reader: impl Read) -> io::Result<usize> {
        let mut buf = BorrowedBuf::from(&mut self.buf[self.filled..]);
        let old_init = self.initialized - self.filled;
        unsafe {
             buf.set_init(old_init);
        }
        reader.read_buf(buf.unfilled())?;
        self.filled += buf.len();
        self.initialized += buf.init_len() - old_init;
        Ok(buf.len())
    }

I think the unsafe block in the read_more() might be unused because I was taking a look into what reader.read_buf() does and it seems to already be setting the BorrowBuf's init field to another value.

fn read_buf(&mut self, buf: BorrowedCursor<'_>) -> Result<()> {
        default_read_buf(|b| self.read(b), buf)
}

pub(crate) fn default_read_buf<F>(read: F, mut cursor: BorrowedCursor<'_>) -> Result<()>
where
    F: FnOnce(&mut [u8]) -> Result<usize>,
{
    let n = read(cursor.ensure_init().init_mut())?;
    cursor.advance(n);
    Ok(())
}

/// Initializes all bytes in the cursor.
#[inline]
pub fn ensure_init(&mut self) -> &mut Self {
    // SAFETY: always in bounds and we never uninitialize these bytes.
    let uninit = unsafe { self.buf.buf.get_unchecked_mut(self.buf.init..) };

    // SAFETY: 0 is a valid value for MaybeUninit<u8> and the length matches the allocation
    // since it is comes from a slice reference.
    unsafe {
        ptr::write_bytes(uninit.as_mut_ptr(), 0, uninit.len());
    }
    self.buf.init = self.buf.capacity();

    self
}

It seems like the init field is reassigned inside the ensure_init() function, which would make the buf.set_init(old_init) code not useful. I know that the buf.unfilled() that is passed into reader.read_buf() is a BorrowedCursor<'this>, but as far as I am aware, a BorrowCursor<'this> is a wrapper around the unfilled portion of BorrowedBuf<'_>:

#[derive(Debug)]
pub struct BorrowedCursor<'a> {
    /// The underlying buffer.
    // Safety invariant: we treat the type of buf as covariant in the lifetime of `BorrowedBuf` when
    // we create a `BorrowedCursor`. This is only safe if we never replace `buf` by assigning into
    // it, so don't do that!
    buf: &'a mut BorrowedBuf<'a>,
}

If I'm understanding things correctly, I believe it's fine to get rid of the unsafe block code in this function.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 17, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2026

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@asder8215 asder8215 changed the title removed unnecessary unsafe block code in Buffer's read_more method remove unnecessary unsafe block code in Buffer's read_more method Jan 17, 2026
@rust-log-analyzer
Copy link
Collaborator

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
REPOSITORY                                   TAG       IMAGE ID       CREATED      SIZE
ghcr.io/dependabot/dependabot-updater-core   latest    63ba4a2ff346   9 days ago   775MB
=> Removing docker images...
Deleted Images:
untagged: ghcr.io/dependabot/dependabot-updater-core:latest
untagged: ghcr.io/dependabot/dependabot-updater-core@sha256:830c840ea4b8c27b6919bdd47c017030adeb538f226e1cdfd386490a622b9218
deleted: sha256:63ba4a2ff3467a98ac6c8610fece2cdc9893e2af6c18111e57618f8949749b82
deleted: sha256:77617fa265a7311b1c0502e01ff6157d25228d91f6cddc21d7901eb440d0adee
deleted: sha256:0bd8abdbb1f7c0453b4bc76cce6ecf0a3b9e4b5d0c8ceeb63652e91a63cab4a1
deleted: sha256:4e9913397e051a70593edc1662de159778ce1bf65132295639da0e119913b53f
deleted: sha256:3e54edb2fef4b1c1393a2928bfa1017c22cc578f5f3eef677580662e8d2e79ea
---
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap (line 72) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,RandomState>::new (line 264) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,RandomState>::with_capacity (line 283) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,RandomState>::from (line 1503) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::capacity (line 445) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::clear (line 819) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::contains_key (line 1220) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::drain (line 721) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::entry (line 959) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::get (line 987) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::extract_if (line 761) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::get_disjoint_mut (line 1077) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::get_disjoint_mut (line 1119) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::hasher (line 837) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::get_key_value (line 1017) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::get_disjoint_unchecked_mut (line 1162) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::get_mut (line 1247) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::insert (line 1280) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::into_iter (line 2052) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::into_keys (line 491) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::into_values (line 586) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::is_empty (line 698) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::iter (line 618) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::keys (line 461) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::iter_mut (line 648) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::len (line 681) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::remove (line 1339) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::reserve (line 870) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::remove_entry (line 1367) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::retain (line 792) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::shrink_to (line 937) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::shrink_to_fit (line 913) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::try_insert (line 1309) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::try_reserve (line 895) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::values (line 523) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S,A>::values_mut (line 552) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S>::with_capacity_and_hasher (line 383) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::HashMap<K,V,S>::with_hasher (line 351) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::IntoKeys (line 1842) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::IntoIter (line 1610) ... ok
test library/std/src/collections/hash/map.rs - collections::hash::map::IntoValues (line 1876) ... ok
---
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,RandomState>::new (line 142) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,RandomState>::with_capacity (line 161) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::capacity (line 313) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,RandomState>::from (line 1180) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::clear (line 490) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::contains (line 758) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::drain (line 398) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::difference (line 630) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::entry (line 861) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::extract_if (line 434) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::get (line 783) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::get_or_insert (line 805) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::hasher (line 508) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::get_or_insert_with (line 829) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::insert (line 983) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::into_iter (line 1642) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::is_disjoint (line 903) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::intersection (line 697) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::is_empty (line 375) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::iter (line 329) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::is_subset (line 929) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::is_superset (line 951) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::len (line 357) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::reserve (line 541) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::remove (line 1030) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::replace (line 1004) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::retain (line 465) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::shrink_to (line 607) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::shrink_to_fit (line 584) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::symmetric_difference (line 660) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::take (line 1058) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::try_reserve (line 567) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S,A>::union (line 726) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S>::with_capacity_and_hasher (line 255) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::HashSet<T,S>::with_hasher (line 223) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::Intersection (line 1506) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::Iter (line 1391) ... ok
test library/std/src/collections/hash/set.rs - collections::hash::set::IntoIter (line 1421) ... ok

@asder8215
Copy link
Contributor Author

I'm not sure if the pr-check-2 is a fault of this PR because the error output isn't related to the change I'm making.

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

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants