From 04a9c7fff1d351bb7ab6d254429ff0a992b0caa4 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Sat, 26 Jun 2021 08:38:55 +0600 Subject: [PATCH] fix tests --- ntex-bytes/Cargo.toml | 1 + ntex-bytes/src/buf/buf_impl.rs | 2 +- ntex-bytes/src/buf/buf_mut.rs | 87 +++++++------- ntex-bytes/src/buf/mod.rs | 2 + ntex-bytes/src/buf/uninit_slice.rs | 187 +++++++++++++++++++++++++++++ ntex-bytes/src/bytes.rs | 27 +++-- ntex-bytes/src/serde.rs | 13 +- ntex-bytes/src/string.rs | 51 ++------ ntex-bytes/tests/test_buf.rs | 6 +- ntex-bytes/tests/test_buf_mut.rs | 12 +- ntex-bytes/tests/test_serde.rs | 8 +- ntex/src/http/h1/encoder.rs | 2 +- 12 files changed, 275 insertions(+), 123 deletions(-) create mode 100644 ntex-bytes/src/buf/uninit_slice.rs diff --git a/ntex-bytes/Cargo.toml b/ntex-bytes/Cargo.toml index 380fa19a..823f6db3 100644 --- a/ntex-bytes/Cargo.toml +++ b/ntex-bytes/Cargo.toml @@ -17,3 +17,4 @@ bytes = "1.0.1" [dev-dependencies] serde_test = "1.0" +serde_json = "1.0" diff --git a/ntex-bytes/src/buf/buf_impl.rs b/ntex-bytes/src/buf/buf_impl.rs index f7d9f9c2..d00c9b12 100644 --- a/ntex-bytes/src/buf/buf_impl.rs +++ b/ntex-bytes/src/buf/buf_impl.rs @@ -106,7 +106,7 @@ pub trait Buf { /// /// let mut buf = &b"hello world"[..]; /// - /// assert_eq!(buf.bytes(), &b"hello world"[..]); + /// assert_eq!(buf.chunk(), &b"hello world"[..]); /// /// buf.advance(6); /// diff --git a/ntex-bytes/src/buf/buf_mut.rs b/ntex-bytes/src/buf/buf_mut.rs index f8de1afb..530ba039 100644 --- a/ntex-bytes/src/buf/buf_mut.rs +++ b/ntex-bytes/src/buf/buf_mut.rs @@ -1,7 +1,7 @@ -use super::Writer; - use std::{cmp, mem, ptr, usize}; +use super::{UninitSlice, Writer}; + /// A trait for values that provide sequential write access to bytes. /// /// Write bytes to a buffer @@ -27,7 +27,7 @@ pub trait BufMut { /// position until the end of the buffer is reached. /// /// This value is greater than or equal to the length of the slice returned - /// by `bytes_mut`. + /// by `chunk_mut`. /// /// # Examples /// @@ -66,14 +66,10 @@ pub trait BufMut { /// let mut buf = Vec::with_capacity(16); /// /// unsafe { - /// buf.bytes_mut()[0] = b'h'; - /// buf.bytes_mut()[1] = b'e'; - /// + /// buf.chunk_mut()[0..2].copy_from_slice(b"he"); /// buf.advance_mut(2); /// - /// buf.bytes_mut()[0] = b'l'; - /// buf.bytes_mut()[1..3].copy_from_slice(b"lo"); - /// + /// buf.chunk_mut()[0..3].copy_from_slice(b"llo"); /// buf.advance_mut(3); /// } /// @@ -134,13 +130,11 @@ pub trait BufMut { /// let mut buf = Vec::with_capacity(16); /// /// unsafe { - /// buf.bytes_mut()[0] = b'h'; - /// buf.bytes_mut()[1] = b'e'; + /// buf.chunk_mut()[0..2].copy_from_slice(b"he"); /// /// buf.advance_mut(2); /// - /// buf.bytes_mut()[0] = b'l'; - /// buf.bytes_mut()[1..3].copy_from_slice(b"lo"); + /// buf.chunk_mut()[0..3].copy_from_slice(b"llo"); /// /// buf.advance_mut(3); /// } @@ -156,7 +150,7 @@ pub trait BufMut { /// `bytes_mut` returning an empty slice implies that `remaining_mut` will /// return 0 and `remaining_mut` returning 0 implies that `bytes_mut` will /// return an empty slice. - unsafe fn chunk_mut(&mut self) -> &mut [u8]; + fn chunk_mut(&mut self) -> &mut UninitSlice; /// Transfer bytes into `self` from `src` and advance the cursor by the /// number of bytes written. @@ -192,7 +186,7 @@ pub trait BufMut { let d = self.chunk_mut(); l = cmp::min(s.len(), d.len()); - ptr::copy_nonoverlapping(s.as_ptr(), d.as_mut_ptr(), l); + ptr::copy_nonoverlapping(s.as_ptr(), d.as_mut_ptr() as *mut u8, l); } src.advance(l); @@ -233,7 +227,11 @@ pub trait BufMut { let dst = self.chunk_mut(); cnt = cmp::min(dst.len(), src.len() - off); - ptr::copy_nonoverlapping(src[off..].as_ptr(), dst.as_mut_ptr(), cnt); + ptr::copy_nonoverlapping( + src[off..].as_ptr(), + dst.as_mut_ptr() as *mut u8, + cnt, + ); off += cnt; } @@ -882,7 +880,7 @@ impl BufMut for &mut T { (**self).remaining_mut() } - unsafe fn chunk_mut(&mut self) -> &mut [u8] { + fn chunk_mut(&mut self) -> &mut UninitSlice { (**self).chunk_mut() } @@ -896,7 +894,7 @@ impl BufMut for Box { (**self).remaining_mut() } - unsafe fn chunk_mut(&mut self) -> &mut [u8] { + fn chunk_mut(&mut self) -> &mut UninitSlice { (**self).chunk_mut() } @@ -905,25 +903,6 @@ impl BufMut for Box { } } -impl BufMut for &mut [u8] { - #[inline] - fn remaining_mut(&self) -> usize { - self.len() - } - - #[inline] - unsafe fn chunk_mut(&mut self) -> &mut [u8] { - self - } - - #[inline] - unsafe fn advance_mut(&mut self, cnt: usize) { - // Lifetime dance taken from `impl Write for &mut [u8]`. - let (_, b) = std::mem::replace(self, &mut []).split_at_mut(cnt); - *self = b; - } -} - impl BufMut for Vec { #[inline] fn remaining_mut(&self) -> usize { @@ -944,9 +923,7 @@ impl BufMut for Vec { } #[inline] - unsafe fn chunk_mut(&mut self) -> &mut [u8] { - use std::slice; - + fn chunk_mut(&mut self) -> &mut UninitSlice { if self.capacity() == self.len() { self.reserve(64); // Grow the vec } @@ -955,7 +932,35 @@ impl BufMut for Vec { let len = self.len(); let ptr = self.as_mut_ptr(); - &mut slice::from_raw_parts_mut(ptr, cap)[len..] + unsafe { &mut UninitSlice::from_raw_parts_mut(ptr, cap)[len..] } + } +} + +impl BufMut for &mut [u8] { + #[inline] + fn remaining_mut(&self) -> usize { + self.len() + } + + #[inline] + fn chunk_mut(&mut self) -> &mut UninitSlice { + // UninitSlice is repr(transparent), so safe to transmute + unsafe { &mut *(*self as *mut [u8] as *mut _) } + } + + #[inline] + unsafe fn advance_mut(&mut self, cnt: usize) { + // Lifetime dance taken from `impl Write for &mut [u8]`. + let (_, b) = core::mem::replace(self, &mut []).split_at_mut(cnt); + *self = b; + } + + #[inline] + fn put_slice(&mut self, src: &[u8]) { + self[..src.len()].copy_from_slice(src); + unsafe { + self.advance_mut(src.len()); + } } } diff --git a/ntex-bytes/src/buf/mod.rs b/ntex-bytes/src/buf/mod.rs index fa73ee81..a30d9e08 100644 --- a/ntex-bytes/src/buf/mod.rs +++ b/ntex-bytes/src/buf/mod.rs @@ -19,9 +19,11 @@ mod buf_impl; mod buf_mut; mod iter; +mod uninit_slice; mod writer; pub use self::buf_impl::Buf; pub use self::buf_mut::BufMut; pub use self::iter::IntoIter; +pub use self::uninit_slice::UninitSlice; pub use self::writer::Writer; diff --git a/ntex-bytes/src/buf/uninit_slice.rs b/ntex-bytes/src/buf/uninit_slice.rs new file mode 100644 index 00000000..d85c098a --- /dev/null +++ b/ntex-bytes/src/buf/uninit_slice.rs @@ -0,0 +1,187 @@ +use core::fmt; +use core::mem::MaybeUninit; +use core::ops::{ + Index, IndexMut, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, + RangeToInclusive, +}; + +/// Uninitialized byte slice. +/// +/// Returned by `BufMut::chunk_mut()`, the referenced byte slice may be +/// uninitialized. The wrapper provides safe access without introducing +/// undefined behavior. +/// +/// The safety invariants of this wrapper are: +/// +/// 1. Reading from an `UninitSlice` is undefined behavior. +/// 2. Writing uninitialized bytes to an `UninitSlice` is undefined behavior. +/// +/// The difference between `&mut UninitSlice` and `&mut [MaybeUninit]` is +/// that it is possible in safe code to write uninitialized bytes to an +/// `&mut [MaybeUninit]`, which this type prohibits. +#[repr(transparent)] +pub struct UninitSlice([MaybeUninit]); + +impl UninitSlice { + /// Create a `&mut UninitSlice` from a pointer and a length. + /// + /// # Safety + /// + /// The caller must ensure that `ptr` references a valid memory region owned + /// by the caller representing a byte slice for the duration of `'a`. + /// + /// # Examples + /// + /// ``` + /// use ntex_bytes::buf::UninitSlice; + /// + /// let bytes = b"hello world".to_vec(); + /// let ptr = bytes.as_ptr() as *mut _; + /// let len = bytes.len(); + /// + /// let slice = unsafe { UninitSlice::from_raw_parts_mut(ptr, len) }; + /// ``` + #[inline] + pub unsafe fn from_raw_parts_mut<'a>( + ptr: *mut u8, + len: usize, + ) -> &'a mut UninitSlice { + let maybe_init: &mut [MaybeUninit] = + core::slice::from_raw_parts_mut(ptr as *mut _, len); + &mut *(maybe_init as *mut [MaybeUninit] as *mut UninitSlice) + } + + /// Write a single byte at the specified offset. + /// + /// # Panics + /// + /// The function panics if `index` is out of bounds. + /// + /// # Examples + /// + /// ``` + /// use ntex_bytes::buf::UninitSlice; + /// + /// let mut data = [b'f', b'o', b'o']; + /// let slice = unsafe { UninitSlice::from_raw_parts_mut(data.as_mut_ptr(), 3) }; + /// + /// slice.write_byte(0, b'b'); + /// + /// assert_eq!(b"boo", &data[..]); + /// ``` + #[inline] + pub fn write_byte(&mut self, index: usize, byte: u8) { + assert!(index < self.len()); + + unsafe { self[index..].as_mut_ptr().write(byte) } + } + + /// Copies bytes from `src` into `self`. + /// + /// The length of `src` must be the same as `self`. + /// + /// # Panics + /// + /// The function panics if `src` has a different length than `self`. + /// + /// # Examples + /// + /// ``` + /// use ntex_bytes::buf::UninitSlice; + /// + /// let mut data = [b'f', b'o', b'o']; + /// let slice = unsafe { UninitSlice::from_raw_parts_mut(data.as_mut_ptr(), 3) }; + /// + /// slice.copy_from_slice(b"bar"); + /// + /// assert_eq!(b"bar", &data[..]); + /// ``` + #[inline] + pub fn copy_from_slice(&mut self, src: &[u8]) { + use core::ptr; + + assert_eq!(self.len(), src.len()); + + unsafe { + ptr::copy_nonoverlapping(src.as_ptr(), self.as_mut_ptr(), self.len()); + } + } + + /// Return a raw pointer to the slice's buffer. + /// + /// # Safety + /// + /// The caller **must not** read from the referenced memory and **must not** + /// write **uninitialized** bytes to the slice either. + /// + /// # Examples + /// + /// ``` + /// use ntex_bytes::BufMut; + /// + /// let mut data = [0, 1, 2]; + /// let mut slice = &mut data[..]; + /// let ptr = BufMut::chunk_mut(&mut slice).as_mut_ptr(); + /// ``` + #[inline] + pub fn as_mut_ptr(&mut self) -> *mut u8 { + self.0.as_mut_ptr() as *mut _ + } + + /// Returns the number of bytes in the slice. + /// + /// # Examples + /// + /// ``` + /// use ntex_bytes::BufMut; + /// + /// let mut data = [0, 1, 2]; + /// let mut slice = &mut data[..]; + /// let len = BufMut::chunk_mut(&mut slice).len(); + /// + /// assert_eq!(len, 3); + /// ``` + #[inline] + pub fn len(&self) -> usize { + self.0.len() + } +} + +impl fmt::Debug for UninitSlice { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("UninitSlice[...]").finish() + } +} + +macro_rules! impl_index { + ($($t:ty),*) => { + $( + impl Index<$t> for UninitSlice { + type Output = UninitSlice; + + #[inline] + fn index(&self, index: $t) -> &UninitSlice { + let maybe_uninit: &[MaybeUninit] = &self.0[index]; + unsafe { &*(maybe_uninit as *const [MaybeUninit] as *const UninitSlice) } + } + } + + impl IndexMut<$t> for UninitSlice { + #[inline] + fn index_mut(&mut self, index: $t) -> &mut UninitSlice { + let maybe_uninit: &mut [MaybeUninit] = &mut self.0[index]; + unsafe { &mut *(maybe_uninit as *mut [MaybeUninit] as *mut UninitSlice) } + } + } + )* + }; +} + +impl_index!( + Range, + RangeFrom, + RangeFull, + RangeInclusive, + RangeTo, + RangeToInclusive +); diff --git a/ntex-bytes/src/bytes.rs b/ntex-bytes/src/bytes.rs index 41efa535..17b1d322 100644 --- a/ntex-bytes/src/bytes.rs +++ b/ntex-bytes/src/bytes.rs @@ -1,7 +1,3 @@ -use crate::buf::IntoIter; -use crate::debug; -use crate::{Buf, BufMut}; - use std::borrow::{Borrow, BorrowMut}; use std::iter::{FromIterator, Iterator}; use std::ops::{Deref, DerefMut, RangeBounds}; @@ -9,6 +5,8 @@ use std::sync::atomic::Ordering::{AcqRel, Acquire, Relaxed, Release}; use std::sync::atomic::{self, AtomicPtr, AtomicUsize}; use std::{cmp, fmt, hash, mem, ptr, slice, usize}; +use crate::{buf::IntoIter, buf::UninitSlice, debug, Buf, BufMut}; + /// A reference counted contiguous slice of memory. /// /// `Bytes` is an efficient container for storing and operating on contiguous @@ -1659,11 +1657,15 @@ impl BufMut for BytesMut { } #[inline] - unsafe fn chunk_mut(&mut self) -> &mut [u8] { + fn chunk_mut(&mut self) -> &mut UninitSlice { let len = self.len(); - // This will never panic as `len` can never become invalid - &mut self.inner.as_raw()[len..] + unsafe { + // This will never panic as `len` can never become invalid + let ptr = &mut self.inner.as_raw()[len..]; + + UninitSlice::from_raw_parts_mut(ptr.as_mut_ptr(), self.capacity() - len) + } } #[inline] @@ -1673,7 +1675,11 @@ impl BufMut for BytesMut { let len = src.len(); unsafe { - self.chunk_mut()[..len].copy_from_slice(src); + ptr::copy_nonoverlapping( + src.as_ptr(), + self.chunk_mut().as_mut_ptr() as *mut u8, + len, + ); self.advance_mut(len); } } @@ -1879,10 +1885,7 @@ impl Extend for BytesMut { self.reserve(lower); for b in iter { - unsafe { - self.chunk_mut()[0] = b; - self.advance_mut(1); - } + self.put_u8(b); } } } diff --git a/ntex-bytes/src/serde.rs b/ntex-bytes/src/serde.rs index b51a024c..08ff6441 100644 --- a/ntex-bytes/src/serde.rs +++ b/ntex-bytes/src/serde.rs @@ -1,9 +1,10 @@ -use super::{Bytes, BytesMut}; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use std::{cmp, fmt}; +use super::{Bytes, BytesMut}; + macro_rules! serde_impl { - ($ty:ident, $visitor_ty:ident) => { + ($ty:ident, $visitor_ty:ident, $from_slice:ident) => { impl Serialize for $ty { #[inline] fn serialize(&self, serializer: S) -> Result @@ -43,7 +44,7 @@ macro_rules! serde_impl { where E: de::Error, { - Ok($ty::from(v)) + Ok($ty::$from_slice(v)) } #[inline] @@ -59,7 +60,7 @@ macro_rules! serde_impl { where E: de::Error, { - Ok($ty::from(v)) + Ok($ty::$from_slice(v.as_bytes())) } #[inline] @@ -83,5 +84,5 @@ macro_rules! serde_impl { }; } -serde_impl!(Bytes, BytesVisitor); -serde_impl!(BytesMut, BytesMutVisitor); +serde_impl!(Bytes, BytesVisitor, copy_from_slice); +serde_impl!(BytesMut, BytesMutVisitor, from); diff --git a/ntex-bytes/src/string.rs b/ntex-bytes/src/string.rs index 39781605..48fb3b40 100644 --- a/ntex-bytes/src/string.rs +++ b/ntex-bytes/src/string.rs @@ -156,32 +156,6 @@ impl TryFrom for ByteString { } } -macro_rules! array_impls { - ($($len:expr)+) => { - $( - impl TryFrom<[u8; $len]> for ByteString { - type Error = str::Utf8Error; - - #[inline] - fn try_from(value: [u8; $len]) -> Result { - ByteString::try_from(&value[..]) - } - } - - impl TryFrom<&[u8; $len]> for ByteString { - type Error = str::Utf8Error; - - #[inline] - fn try_from(value: &[u8; $len]) -> Result { - ByteString::try_from(&value[..]) - } - } - )+ - } -} - -array_impls!(0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32); - impl fmt::Debug for ByteString { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { (**self).fmt(fmt) @@ -196,8 +170,6 @@ impl fmt::Display for ByteString { #[cfg(feature = "serde")] mod serde { - use alloc::string::String; - use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; @@ -226,10 +198,9 @@ mod serde { #[cfg(test)] mod test { - use alloc::borrow::ToOwned; - use core::hash::{Hash, Hasher}; - - use ahash::AHasher; + use std::borrow::ToOwned; + use std::collections::hash_map::DefaultHasher; + use std::hash::{Hash, Hasher}; use super::*; @@ -248,10 +219,10 @@ mod test { #[test] fn test_hash() { - let mut hasher1 = AHasher::default(); + let mut hasher1 = DefaultHasher::default(); "str".hash(&mut hasher1); - let mut hasher2 = AHasher::default(); + let mut hasher2 = DefaultHasher::default(); let s = ByteString::from_static("str"); s.hash(&mut hasher2); assert_eq!(hasher1.finish(), hasher2.finish()); @@ -278,15 +249,7 @@ mod test { #[test] fn test_try_from_slice() { - let _ = ByteString::try_from(b"nice bytes").unwrap(); - } - - #[test] - fn test_try_from_array() { - assert_eq!( - ByteString::try_from([b'h', b'i']).unwrap(), - ByteString::from_static("hi") - ); + let _ = ByteString::try_from(Bytes::from_static(b"nice bytes")).unwrap(); } #[test] @@ -296,7 +259,7 @@ mod test { #[test] fn test_try_from_bytes_mut() { - let _ = ByteString::try_from(bytes::BytesMut::from(&b"nice bytes"[..])).unwrap(); + let _ = ByteString::try_from(crate::BytesMut::from(&b"nice bytes"[..])).unwrap(); } #[cfg(feature = "serde")] diff --git a/ntex-bytes/tests/test_buf.rs b/ntex-bytes/tests/test_buf.rs index fff1215f..f210d747 100644 --- a/ntex-bytes/tests/test_buf.rs +++ b/ntex-bytes/tests/test_buf.rs @@ -7,17 +7,17 @@ fn test_fresh_cursor_vec() { let mut buf = &b"hello"[..]; assert_eq!(buf.remaining(), 5); - assert_eq!(buf.bytes(), b"hello"); + assert_eq!(buf.chunk(), b"hello"); buf.advance(2); assert_eq!(buf.remaining(), 3); - assert_eq!(buf.bytes(), b"llo"); + assert_eq!(buf.chunk(), b"llo"); buf.advance(3); assert_eq!(buf.remaining(), 0); - assert_eq!(buf.bytes(), b""); + assert_eq!(buf.chunk(), b""); } #[test] diff --git a/ntex-bytes/tests/test_buf_mut.rs b/ntex-bytes/tests/test_buf_mut.rs index 19abaa50..37d80f98 100644 --- a/ntex-bytes/tests/test_buf_mut.rs +++ b/ntex-bytes/tests/test_buf_mut.rs @@ -9,10 +9,7 @@ fn test_vec_as_mut_buf() { let mut buf = Vec::with_capacity(64); assert_eq!(buf.remaining_mut(), usize::MAX); - - unsafe { - assert!(buf.bytes_mut().len() >= 64); - } + assert!(buf.chunk_mut().len() >= 64); buf.put(&b"zomg"[..]); @@ -66,10 +63,3 @@ fn test_clone() { buf.write_str(" of our emergency broadcast system").unwrap(); assert!(buf != buf2); } - -#[test] -fn test_mut_slice() { - let mut v = vec![0, 0, 0, 0]; - let mut s = &mut v[..]; - s.put_u32(42); -} diff --git a/ntex-bytes/tests/test_serde.rs b/ntex-bytes/tests/test_serde.rs index 36b87f28..ab585437 100644 --- a/ntex-bytes/tests/test_serde.rs +++ b/ntex-bytes/tests/test_serde.rs @@ -5,16 +5,16 @@ use serde_test::{assert_tokens, Token}; #[test] fn test_ser_de_empty() { - let b = bytes::Bytes::new(); + let b = ntex_bytes::Bytes::new(); assert_tokens(&b, &[Token::Bytes(b"")]); - let b = bytes::BytesMut::with_capacity(0); + let b = ntex_bytes::BytesMut::with_capacity(0); assert_tokens(&b, &[Token::Bytes(b"")]); } #[test] fn test_ser_de() { - let b = bytes::Bytes::from(&b"bytes"[..]); + let b = ntex_bytes::Bytes::from(&b"bytes"[..]); assert_tokens(&b, &[Token::Bytes(b"bytes")]); - let b = bytes::BytesMut::from(&b"bytes"[..]); + let b = ntex_bytes::BytesMut::from(&b"bytes"[..]); assert_tokens(&b, &[Token::Bytes(b"bytes")]); } diff --git a/ntex/src/http/h1/encoder.rs b/ntex/src/http/h1/encoder.rs index a2c235a6..ec33a2ea 100644 --- a/ntex/src/http/h1/encoder.rs +++ b/ntex/src/http/h1/encoder.rs @@ -114,7 +114,7 @@ pub(super) trait MessageType: Sized { let mut pos = 0; let mut has_date = false; let mut remaining = dst.capacity() - dst.len(); - let mut buf = unsafe { dst.chunk_mut().as_mut_ptr() as *mut u8 }; + let mut buf = dst.chunk_mut().as_mut_ptr() as *mut u8; for (key, value) in headers { match *key { CONNECTION => continue,