Fix errors found by Clang warning -Wdangling-gsl
This warning detects when pointers are owned by an object, but that object is deleted at the end of a statement. Since the pointer is to freed memory, accessing the pointer may produce unexpected results.
For more information on this cleanup: go/cleanup-wdangling-gsl
Examples:
std::unique_ptr<int> getPtr();
void foo() {
int* ptr = getPtr().get();
// The unique_ptr owns the pointer, but the unique_ptr’s lifetime ends
// at the end of the statement.
std::unique_ptr<int> owned = getPtr();
int* ptr = owned.get();
// Capturing the unique_ptr will keep the pointer valid as long as
// the unique_ptr is in scope.
}
std::string getString();
void foo() {
absl::string_view str = getString();
// string_view does not own the backing data, so it is only valid until
// the end of the statement, where the std::string is destroyed.
std::string str = getString();
// Capture the string to a local variable.
// Alternatively, make getString() return a string reference, if the
// string’s lifetime extends beyond the function getString.
}
const std::string& getString();
void foo(bool b) {
absl::string_view str = b ? getString() : “default”;
// Initializing a string_view with either a const string reference
// or a string literal would be valid. However, the conditional
// operator forces the two operands to have the same type first.
// Explicitly writing this out would be:
// absl::string_view str = b ? getString()
// : std::string(“default”);
// The string constructed from the string literal gets destroyed
// at the end of the statement.
absl::string_view str b ? absl::string_view(getString())
: absl::string_view(“default”);
// Use string_view constructors instead.
}
std::vector<std::string> getVector();
void foo() {
const std::string& str = getVector()[0];
// Container access will return a reference to elements. When
// the returned container is deleted, the references to its
// elements are invalid.
const std::string str = getVector()[0];
// Make a copy of the container element.
std::vector<std::string> vector = getVector();
const std::string& str = vector[0];
// Save the returned container.
// Alternatively, make getVector return a reference to the container
// if the returned container outlives the function call.
}
For more information on this cleanup: go/cleanup-wdangling-gsl
Tested:
TAP --sample ran all affected tests and none failed
http://test/OCL:312589380:BASE:312637687:1590054022610:cb8287b
PiperOrigin-RevId: 313171633
Change-Id: I6cd6f0e8a7acab0bbdc4383f5a9b04feea6876f4
diff --git a/quic/core/quic_versions_test.cc b/quic/core/quic_versions_test.cc
index 52278f7..1ca9fae 100644
--- a/quic/core/quic_versions_test.cc
+++ b/quic/core/quic_versions_test.cc
@@ -555,10 +555,11 @@
QuicTransportVersionVector all_versions = {QUIC_VERSION_43};
int version_count = all_versions.size();
for (int i = -5; i <= version_count + 1; ++i) {
+ QuicTransportVersionVector index = VersionOfIndex(all_versions, i);
if (i >= 0 && i < version_count) {
- EXPECT_EQ(all_versions[i], VersionOfIndex(all_versions, i)[0]);
+ EXPECT_EQ(all_versions[i], index[0]);
} else {
- EXPECT_EQ(QUIC_VERSION_UNSUPPORTED, VersionOfIndex(all_versions, i)[0]);
+ EXPECT_EQ(QUIC_VERSION_UNSUPPORTED, index[0]);
}
}
}
@@ -567,11 +568,11 @@
ParsedQuicVersionVector all_versions = AllSupportedVersions();
int version_count = all_versions.size();
for (int i = -5; i <= version_count + 1; ++i) {
+ ParsedQuicVersionVector index = ParsedVersionOfIndex(all_versions, i);
if (i >= 0 && i < version_count) {
- EXPECT_EQ(all_versions[i], ParsedVersionOfIndex(all_versions, i)[0]);
+ EXPECT_EQ(all_versions[i], index[0]);
} else {
- EXPECT_EQ(UnsupportedQuicVersion(),
- ParsedVersionOfIndex(all_versions, i)[0]);
+ EXPECT_EQ(UnsupportedQuicVersion(), index[0]);
}
}
}
@@ -704,7 +705,8 @@
SupportedTransportVersions()) {
SCOPED_TRACE(index);
if (ParsedQuicVersionIsValid(handshake_protocol, transport_version)) {
- EXPECT_EQ(SupportedVersions()[index],
+ ParsedQuicVersion version = SupportedVersions()[index];
+ EXPECT_EQ(version,
ParsedQuicVersion(handshake_protocol, transport_version));
index++;
}
diff --git a/quic/core/tls_chlo_extractor_test.cc b/quic/core/tls_chlo_extractor_test.cc
index ba57ad0..955e58c 100644
--- a/quic/core/tls_chlo_extractor_test.cc
+++ b/quic/core/tls_chlo_extractor_test.cc
@@ -48,8 +48,9 @@
void ValidateChloDetails() {
EXPECT_TRUE(tls_chlo_extractor_.HasParsedFullChlo());
- ASSERT_EQ(tls_chlo_extractor_.alpns().size(), 1u);
- EXPECT_EQ(tls_chlo_extractor_.alpns()[0], AlpnForVersion(version_));
+ std::vector<std::string> alpns = tls_chlo_extractor_.alpns();
+ ASSERT_EQ(alpns.size(), 1u);
+ EXPECT_EQ(alpns[0], AlpnForVersion(version_));
EXPECT_EQ(tls_chlo_extractor_.server_name(), TestHostname());
}
diff --git a/quic/quartc/quartc_fakes.h b/quic/quartc/quartc_fakes.h
index 94bf1ad..774cd09 100644
--- a/quic/quartc/quartc_fakes.h
+++ b/quic/quartc/quartc_fakes.h
@@ -148,7 +148,7 @@
void OnBufferChanged(QuartcStream* /*stream*/) override {}
bool has_data() { return !received_data_.empty(); }
- std::map<QuicStreamId, std::string> data() { return received_data_; }
+ std::map<QuicStreamId, std::string>& data() { return received_data_; }
QuicRstStreamErrorCode stream_error(QuicStreamId id) { return errors_[id]; }
