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

Include relevant values in ReadBuf.put_slice() panic message #6580

Closed
OmegaJak opened this issue May 22, 2024 · 1 comment
Closed

Include relevant values in ReadBuf.put_slice() panic message #6580

OmegaJak opened this issue May 22, 2024 · 1 comment
Labels
A-tokio Area: The main tokio crate C-feature-accepted Category: A feature request that has been accepted pending implementation. C-feature-request Category: A feature request. M-io Module: tokio/io

Comments

@OmegaJak
Copy link

OmegaJak commented May 22, 2024

Is your feature request related to a problem? Please describe.
A service I work on panicked due to the assert at the top of io::ReadBuf::put_slice(), however, the panic message did not include the values that were compared, which would help with debugging the problem.

Describe the solution you'd like
If calling put_slice() panics as a result the remaining space in the ReadBuf being less than the slice's length, it would be nice for the panic message to include the values of self.remaining() and buf.len(). This is the assert in question. I would propose updating it to something like

assert!(
    self.remaining() >= buf.len(),
    "buf.len() must fit in remaining(); buf.len() = {}, remaining() = {}",
    buf.len(),
    self.remaining()
);

Describe alternatives you've considered
Additional logging of these values can be done by users of this function. I suppose it could also return a Result with an error that contains the mismatch but that's a much bigger change that probably has its own downsides.

Additional context
There does seem to be a number of these asserts scattered around tokio and I couldn't find any that included the errant values in the message for a greater than/less than check. If including the values in the error message is a desirable behavior, then it may be worth thinking about adding/using assert macros like assert_eq! for the various greater than/less than comparisons (assert_ge!, assert_le!, etc).

@OmegaJak OmegaJak added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels May 22, 2024
@Darksonn Darksonn added the M-io Module: tokio/io label May 26, 2024
@Darksonn
Copy link
Contributor

This seems like a good idea.

@Darksonn Darksonn added the C-feature-accepted Category: A feature request that has been accepted pending implementation. label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-accepted Category: A feature request that has been accepted pending implementation. C-feature-request Category: A feature request. M-io Module: tokio/io
Projects
None yet
Development

No branches or pull requests

2 participants