replace strncpy with strlcpy #207
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattklein123
reviewed
Nov 9, 2016
| @@ -69,6 +69,12 @@ class StringUtil { | |||
| static void rtrim(std::string& source); | |||
|
|
|||
| /** | |||
| * size-bounded string copying and concatenation | |||
| * Adapted from https://github.com/freebsd/freebsd/blob/20c3c08/sys/libkern/strlcpy.c | |||
There was a problem hiding this comment.
nit: move this comment (where adapted from) into the implementation file.
mattklein123
reviewed
Nov 9, 2016
| @@ -1,4 +1,5 @@ | |||
| #include "stats_impl.h" | |||
| #include "common/common/utility.h" | |||
There was a problem hiding this comment.
nit: newline before this line
mattklein123
reviewed
Nov 9, 2016
| @@ -1,3 +1,5 @@ | |||
| #include "gtest/gtest.h" | |||
mattklein123
reviewed
Nov 9, 2016
| // TODO: more tests | ||
| char dest[6]; | ||
| StringUtil::strlcpy(dest, std::string{"hello"}.c_str(), sizeof(dest)); | ||
| ASSERT_THAT("hello", testing::ContainerEq(dest)); |
ccaraman
reviewed
Nov 10, 2016
| /** | ||
| * strlcpy adapted from https://github.com/freebsd/freebsd/blob/20c3c08/sys/libkern/strlcpy.c | ||
| */ | ||
| size_t StringUtil::strlcpy(char* dst, const char* src, size_t siz) { |
There was a problem hiding this comment.
nit: can you use full words for variables?
| @@ -69,6 +69,11 @@ class StringUtil { | |||
| static void rtrim(std::string& source); | |||
|
|
|||
| /** | |||
| * Size-bounded string copying and concatenation | |||
| */ | |||
| static size_t strlcpy(char* dst, const char* src, size_t siz); | |||
| @@ -51,6 +51,12 @@ void StringUtil::rtrim(std::string& source) { | |||
| } | |||
| } | |||
|
|
|||
| size_t StringUtil::strlcpy(char* dst, const char* src, size_t siz) { | |||
| size_t result; | ||
| result = StringUtil::strlcpy(dest, std::string{"hello"}.c_str(), sizeof(dest)); | ||
| EXPECT_STREQ("hello", dest); | ||
| EXPECT_EQ(5U, result); |
There was a problem hiding this comment.
nit: I would just put the EXPECT_EQ call directly around the strlcpy call since you don't use result anywhere else.
added 2 commits
November 29, 2016 09:45
mattklein123
approved these changes
Nov 29, 2016
rshriram
pushed a commit
to rshriram/envoy
that referenced
this pull request
Oct 30, 2018
Do not use request start time as the operation start time when reporting the operation to service control. Service control requires the operation time to be recent (on the order of seconds). Given that a request can have arbitrary latency, the start time of the request might be arbitrarily old.
rshriram
pushed a commit
to rshriram/envoy
that referenced
this pull request
Oct 30, 2018
PiotrSikora
pushed a commit
to PiotrSikora/envoy
that referenced
this pull request
May 12, 2020
* Fetch .wasm from remote URI without depending on Listener. (envoyproxy#204) * Fetch .wasm from remote URI without depending on Listener. Signed-off-by: John Plevyak <[email protected]> * Reactivate tests. Signed-off-by: John Plevyak <[email protected]> * Add stats for wasm remote load fetch and cache. (envoyproxy#207) * Add stats for wasm remote load fetch and cache. Signed-off-by: John Plevyak <[email protected]> * Address comments and ensure that the stats have the same lifetime as the cache. Signed-off-by: John Plevyak <[email protected]> * Address comments. Signed-off-by: John Plevyak <[email protected]> * Address ASAN issue. Signed-off-by: John Plevyak <[email protected]> * Mess around with the tests some more. Signed-off-by: John Plevyak <[email protected]> Co-authored-by: John Plevyak <[email protected]>
gargnupur
pushed a commit
to gargnupur/envoy
that referenced
this pull request
May 13, 2020
* Add stats for wasm remote load fetch and cache. Signed-off-by: John Plevyak <[email protected]> * Address comments and ensure that the stats have the same lifetime as the cache. Signed-off-by: John Plevyak <[email protected]> * Address comments. Signed-off-by: John Plevyak <[email protected]> * Address ASAN issue. Signed-off-by: John Plevyak <[email protected]> * Mess around with the tests some more. Signed-off-by: John Plevyak <[email protected]>
mattklein123
pushed a commit
that referenced
this pull request
Sep 29, 2020
The wee v8 build times out in CI under --config=asan because the machine the job is scheduled on is too small. Signed-off-by: Antonio Vicente <[email protected]>
esmet
pushed a commit
to datawire/envoy
that referenced
this pull request
Apr 15, 2021
…oyproxy#207) The wee v8 build times out in CI under --config=asan because the machine the job is scheduled on is too small. Signed-off-by: Antonio Vicente <[email protected]>
arminabf
pushed a commit
to arminabf/envoy
that referenced
this pull request
Jun 5, 2024
Co-authored-by: Crypt Keeper <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.