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

replace strncpy with strlcpy #207

Merged
merged 5 commits into from
Nov 29, 2016
Merged

replace strncpy with strlcpy #207

merged 5 commits into from
Nov 29, 2016

Conversation

danielhochman
Copy link
Contributor

No description provided.

@@ -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
Copy link
Member

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"
Copy link
Member

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"
Copy link
Member

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));
Copy link
Member

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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:size

Copy link
Contributor

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);
Copy link
Member

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) {
Copy link
Member

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);
Copy link
Member

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.

@mattklein123 mattklein123 merged commit b1a5de6 into master Nov 29, 2016
@mattklein123 mattklein123 deleted the strlcpy branch November 29, 2016 19:44
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 <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>
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 <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>
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 <avd@google.com>
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 <avd@google.com>
arminabf pushed a commit to arminabf/envoy that referenced this pull request Jun 5, 2024
Co-authored-by: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com>
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