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.

Sign up for GitHub

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

Jump to bottom

Add length_field_type to LengthDelimitedCodec builder #4508

Merged
merged 3 commits into from
Feb 23, 2022
Merged

Add length_field_type to LengthDelimitedCodec builder #4508

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3787b1
Add `length_field_type` builder method
nvzqz Feb 16, 2022
511e91b
Add example that fails to compile
nvzqz Feb 17, 2022
2c39667
Use more analogous type in example
nvzqz Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 66 additions & 12 deletions tokio-util/src/codec/length_delimited.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
//! # fn bind_read<T: AsyncRead>(io: T) {
//! LengthDelimitedCodec::builder()
//! .length_field_offset(0) // default value
//! .length_field_length(2)
//! .length_field_type::<u16>()
//! .length_adjustment(0) // default value
//! .num_skip(0) // Do not strip frame header
//! .new_read(io);
Expand Down Expand Up @@ -118,7 +118,7 @@
//! # fn bind_read<T: AsyncRead>(io: T) {
//! LengthDelimitedCodec::builder()
//! .length_field_offset(0) // default value
//! .length_field_length(2)
//! .length_field_type::<u16>()
//! .length_adjustment(0) // default value
//! // `num_skip` is not needed, the default is to skip
//! .new_read(io);
Expand Down Expand Up @@ -150,7 +150,7 @@
//! # fn bind_read<T: AsyncRead>(io: T) {
//! LengthDelimitedCodec::builder()
//! .length_field_offset(0) // default value
//! .length_field_length(2)
//! .length_field_type::<u16>()
//! .length_adjustment(-2) // size of head
//! .num_skip(0)
//! .new_read(io);
Expand Down Expand Up @@ -228,7 +228,7 @@
//! # fn bind_read<T: AsyncRead>(io: T) {
//! LengthDelimitedCodec::builder()
//! .length_field_offset(1) // length of hdr1
//! .length_field_length(2)
//! .length_field_type::<u16>()
//! .length_adjustment(1) // length of hdr2
//! .num_skip(3) // length of hdr1 + LEN
//! .new_read(io);
Expand Down Expand Up @@ -274,7 +274,7 @@
//! # fn bind_read<T: AsyncRead>(io: T) {
//! LengthDelimitedCodec::builder()
//! .length_field_offset(1) // length of hdr1
//! .length_field_length(2)
//! .length_field_type::<u16>()
//! .length_adjustment(-3) // length of hdr1 + LEN, negative
//! .num_skip(3)
//! .new_read(io);
Expand Down Expand Up @@ -350,7 +350,7 @@
//! # fn write_frame<T: AsyncWrite>(io: T) {
//! # let _ =
//! LengthDelimitedCodec::builder()
//! .length_field_length(2)
//! .length_field_type::<u16>()
//! .new_write(io);
//! # }
//! # pub fn main() {}
Expand Down Expand Up @@ -379,7 +379,7 @@ use tokio::io::{AsyncRead, AsyncWrite};
use bytes::{Buf, BufMut, Bytes, BytesMut};
use std::error::Error as StdError;
use std::io::{self, Cursor};
use std::{cmp, fmt};
use std::{cmp, fmt, mem};

/// Configure length delimited `LengthDelimitedCodec`s.
///
Expand Down Expand Up @@ -629,6 +629,24 @@ impl Default for LengthDelimitedCodec {

// ===== impl Builder =====

mod builder {
/// Types that can be used with `Builder::length_field_type`.
pub trait LengthFieldType {}

impl LengthFieldType for u8 {}
impl LengthFieldType for u16 {}
impl LengthFieldType for u32 {}
impl LengthFieldType for u64 {}

#[cfg(any(
target_pointer_width = "8",
target_pointer_width = "16",
target_pointer_width = "32",
target_pointer_width = "64",
))]
impl LengthFieldType for usize {}
}

impl Builder {
/// Creates a new length delimited codec builder with default configuration
/// values.
Expand All @@ -642,7 +660,7 @@ impl Builder {
/// # fn bind_read<T: AsyncRead>(io: T) {
/// LengthDelimitedCodec::builder()
/// .length_field_offset(0)
/// .length_field_length(2)
/// .length_field_type::<u16>()
/// .length_adjustment(0)
/// .num_skip(0)
/// .new_read(io);
Expand Down Expand Up @@ -777,6 +795,42 @@ impl Builder {
self
}

/// Sets the unsigned integer type used to represent the length field.
///
/// The default type is [`u32`]. The max type is [`u64`] (or [`usize`] on
/// 64-bit targets).
Comment on lines +800 to +801
Copy link
Contributor

Choose a reason for hiding this comment

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

The example uses u128?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The compilation failure example uses u128.

///
/// # Examples
///
/// ```
/// # use tokio::io::AsyncRead;
/// use tokio_util::codec::LengthDelimitedCodec;
///
/// # fn bind_read<T: AsyncRead>(io: T) {
/// LengthDelimitedCodec::builder()
/// .length_field_type::<u32>()
/// .new_read(io);
/// # }
/// # pub fn main() {}
Copy link
Member

Choose a reason for hiding this comment

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

my guess is that the fn main()s in these doctests are here because other functions in the module have them? AFAICT, they shouldn't actually be necessary; not a blocker though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I figured the same but wanted to just be consistent with the other doctests.

/// ```
///
/// Unlike [`Builder::length_field_length`], this does not fail at runtime
/// and instead produces a compile error:
///
/// ```compile_fail
/// # use tokio::io::AsyncRead;
/// # use tokio_util::codec::LengthDelimitedCodec;
/// # fn bind_read<T: AsyncRead>(io: T) {
/// LengthDelimitedCodec::builder()
/// .length_field_type::<u128>()
/// .new_read(io);
/// # }
/// # pub fn main() {}
/// ```
pub fn length_field_type<T: builder::LengthFieldType>(&mut self) -> &mut Self {
self.length_field_length(mem::size_of::<T>())
}

/// Sets the number of bytes used to represent the length field
///
/// The default value is `4`. The max value is `8`.
Expand Down Expand Up @@ -878,7 +932,7 @@ impl Builder {
/// # pub fn main() {
/// LengthDelimitedCodec::builder()
/// .length_field_offset(0)
/// .length_field_length(2)
/// .length_field_type::<u16>()
/// .length_adjustment(0)
/// .num_skip(0)
/// .new_codec();
Expand All @@ -902,7 +956,7 @@ impl Builder {
/// # fn bind_read<T: AsyncRead>(io: T) {
/// LengthDelimitedCodec::builder()
/// .length_field_offset(0)
/// .length_field_length(2)
/// .length_field_type::<u16>()
/// .length_adjustment(0)
/// .num_skip(0)
/// .new_read(io);
Expand All @@ -925,7 +979,7 @@ impl Builder {
/// # use tokio_util::codec::LengthDelimitedCodec;
/// # fn write_frame<T: AsyncWrite>(io: T) {
/// LengthDelimitedCodec::builder()
/// .length_field_length(2)
/// .length_field_type::<u16>()
/// .new_write(io);
/// # }
/// # pub fn main() {}
Expand All @@ -947,7 +1001,7 @@ impl Builder {
/// # fn write_frame<T: AsyncRead + AsyncWrite>(io: T) {
/// # let _ =
/// LengthDelimitedCodec::builder()
/// .length_field_length(2)
/// .length_field_type::<u16>()
/// .new_framed(io);
/// # }
/// # pub fn main() {}
Expand Down