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

buffer: add a method for getting only the first slice #14050

Merged
merged 3 commits into from
Nov 19, 2020

Conversation

ggreenway
Copy link
Contributor

Signed-off-by: Greg Greenway ggreenway@apple.com

Commit Message:

Additional Description: This is a minor cleanup in this commit, but more importantly it will
be used in future changes to code in the performance critical path.

Risk Level: Low
Testing: All existing tests pass; added small new unittest
Docs Changes: none; internal change
Release Notes: none; internal change
Platform Specific Features: none
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

This is a minor cleanup in this commit, but more importantly it will
be used in future changes to code in the performance critical path.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
jmarantz
jmarantz previously approved these changes Nov 19, 2020
@ggreenway
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14050 (comment) was created by @ggreenway.

see: more, trace.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
I don't understand why this PR is changing coverage in source/server;
it doesn't touch any files there, and should affect any calls into
that code either.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
@ggreenway ggreenway merged commit 928a62b into envoyproxy:master Nov 19, 2020
andreyprezotto pushed a commit to andreyprezotto/envoy that referenced this pull request Nov 24, 2020
Signed-off-by: Greg Greenway <ggreenway@apple.com>
qqustc pushed a commit to qqustc/envoy that referenced this pull request Nov 24, 2020
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Qin Qin <qqin@google.com>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Some post-submit drive-by comments.

@@ -126,6 +126,8 @@ class StringBuffer : public Buffer::Instance {
return {{const_cast<char*>(start()), size_}};
}

Buffer::RawSlice frontSlice() const override { return {const_cast<char*>(start()), size_}; }
Copy link
Member

Choose a reason for hiding this comment

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

It might be good to have the fuzzer actually call into this method, in addition to defining a fuzz variant.

@@ -1256,6 +1256,13 @@ TEST_F(OwnedImplTest, MoveSmallSliceIntoNotEnoughFreeSpace) {
TestBufferMove(4096 - 127, 128, 2);
}

TEST_F(OwnedImplTest, FrontSlice) {
Buffer::OwnedImpl buffer;
Copy link
Member

Choose a reason for hiding this comment

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

Are there any tests that cover the case when there are leading zero length slices?

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