-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
replace strncpy with strlcpy #207
Conversation
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: move this comment (where adapted from) into the implementation file.
@@ -1,4 +1,5 @@ | |||
#include "stats_impl.h" | |||
#include "common/common/utility.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: newline before this line
@@ -1,3 +1,5 @@ | |||
#include "gtest/gtest.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete
// TODO: more tests | ||
char dest[6]; | ||
StringUtil::strlcpy(dest, std::string{"hello"}.c_str(), sizeof(dest)); | ||
ASSERT_THAT("hello", testing::ContainerEq(dest)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EXPECT_STREQ
/** | ||
* 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: siz -> size
@@ -51,6 +51,12 @@ void StringUtil::rtrim(std::string& source) { | |||
} | |||
} | |||
|
|||
size_t StringUtil::strlcpy(char* dst, const char* src, size_t siz) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: siz -> size
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I would just put the EXPECT_EQ call directly around the strlcpy call since you don't use result anywhere else.
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.
* 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 <jplevyak@gmail.com> * Reactivate tests. Signed-off-by: John Plevyak <jplevyak@gmail.com> * 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 <jplevyak@gmail.com> * Address comments and ensure that the stats have the same lifetime as the cache. Signed-off-by: John Plevyak <jplevyak@gmail.com> * Address comments. Signed-off-by: John Plevyak <jplevyak@gmail.com> * Address ASAN issue. Signed-off-by: John Plevyak <jplevyak@gmail.com> * Mess around with the tests some more. Signed-off-by: John Plevyak <jplevyak@gmail.com> Co-authored-by: John Plevyak <jplevyak@gmail.com>
* Add stats for wasm remote load fetch and cache. Signed-off-by: John Plevyak <jplevyak@gmail.com> * Address comments and ensure that the stats have the same lifetime as the cache. Signed-off-by: John Plevyak <jplevyak@gmail.com> * Address comments. Signed-off-by: John Plevyak <jplevyak@gmail.com> * Address ASAN issue. Signed-off-by: John Plevyak <jplevyak@gmail.com> * Mess around with the tests some more. Signed-off-by: John Plevyak <jplevyak@gmail.com>
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 <avd@google.com>
…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 <avd@google.com>
Co-authored-by: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com>
No description provided.