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

clang-tidy fixes #7318

Merged
merged 1 commit into from
Jun 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Checks: 'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements,readability-container-size-empty'

#TODO(lizan): grow this list, fix possible warnings and make more checks as error
WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements,readability-redundant-string-cstr,bugprone-use-after-move,readability-container-size-empty'
WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements,readability-redundant-string-cstr,bugprone-use-after-move,readability-container-size-empty,performance-faster-string-find'

CheckOptions:
- key: bugprone-assert-side-effect.AssertMacros
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/event/timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ using SchedulerPtr = std::unique_ptr<Scheduler>;
*/
class TimeSystem : public TimeSource {
public:
virtual ~TimeSystem() = default;
~TimeSystem() override = default;

using Duration = MonotonicTime::duration;

Expand Down
9 changes: 3 additions & 6 deletions include/envoy/http/header_map.h
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
#pragma once

#include <string.h>

#include <algorithm>
#include <cstdint>
#include <cstring>
#include <iostream>
#include <memory>
#include <string>
Expand Down Expand Up @@ -133,9 +132,7 @@ class HeaderString {
*
* @return an absl::string_view.
*/
absl::string_view getStringView() const {
return absl::string_view(buffer_.ref_, string_length_);
}
absl::string_view getStringView() const { return {buffer_.ref_, string_length_}; }

/**
* Return the string to a default state. Reference strings are not touched. Both inline/dynamic
Expand Down Expand Up @@ -478,7 +475,7 @@ class HeaderMap {
* @param context supplies the context passed to iterate().
* @return Iterate::Continue to continue iteration.
*/
typedef Iterate (*ConstIterateCb)(const HeaderEntry& header, void* context);
using ConstIterateCb = Iterate (*)(const HeaderEntry&, void*);

/**
* Iterate over a constant header map.
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/registry/registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ template <class Base> class FactoryRegistry {
* Gets the current map of factory implementations. This is an ordered map for sorting reasons.
*/
static std::map<std::string, Base*>& factories() {
static std::map<std::string, Base*>* factories = new std::map<std::string, Base*>;
static auto* factories = new std::map<std::string, Base*>;
return *factories;
}

Expand Down
2 changes: 1 addition & 1 deletion include/envoy/stats/timespan.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ template <class TimeUnit> class TimespanWithUnit : public CompletableTimespan {
/**
* Complete the timespan and send the time to the histogram.
*/
void complete() { histogram_.recordValue(getRawDuration().count()); }
void complete() override { histogram_.recordValue(getRawDuration().count()); }

/**
* Get duration in the time unit since the creation of the span.
Expand Down
3 changes: 1 addition & 2 deletions include/envoy/upstream/types.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#pragma once

#include <stdint.h>

#include <cstdint>
#include <vector>

#include "common/common/phantom.h"
Expand Down
10 changes: 5 additions & 5 deletions source/common/access_log/access_log_formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,31 +238,31 @@ std::vector<FormatterProviderPtr> AccessLogFormatParser::parse(const std::string
pos += 1;
int command_end_position = pos + token.length();

if (token.find("REQ(") == 0) {
if (absl::StartsWith(token, "REQ(")) {
std::string main_header, alternative_header;
absl::optional<size_t> max_length;

parseCommandHeader(token, ReqParamStart, main_header, alternative_header, max_length);

formatters.emplace_back(FormatterProviderPtr{
new RequestHeaderFormatter(main_header, alternative_header, max_length)});
} else if (token.find("RESP(") == 0) {
} else if (absl::StartsWith(token, "RESP(")) {
std::string main_header, alternative_header;
absl::optional<size_t> max_length;

parseCommandHeader(token, RespParamStart, main_header, alternative_header, max_length);

formatters.emplace_back(FormatterProviderPtr{
new ResponseHeaderFormatter(main_header, alternative_header, max_length)});
} else if (token.find("TRAILER(") == 0) {
} else if (absl::StartsWith(token, "TRAILER(")) {
std::string main_header, alternative_header;
absl::optional<size_t> max_length;

parseCommandHeader(token, TrailParamStart, main_header, alternative_header, max_length);

formatters.emplace_back(FormatterProviderPtr{
new ResponseTrailerFormatter(main_header, alternative_header, max_length)});
} else if (token.find(DYNAMIC_META_TOKEN) == 0) {
} else if (absl::StartsWith(token, DYNAMIC_META_TOKEN)) {
std::string filter_namespace;
absl::optional<size_t> max_length;
std::vector<std::string> path;
Expand All @@ -271,7 +271,7 @@ std::vector<FormatterProviderPtr> AccessLogFormatParser::parse(const std::string
parseCommand(token, start, ":", filter_namespace, path, max_length);
formatters.emplace_back(
FormatterProviderPtr{new DynamicMetadataFormatter(filter_namespace, path, max_length)});
} else if (token.find("START_TIME") == 0) {
} else if (absl::StartsWith(token, "START_TIME")) {
const size_t parameters_length = pos + StartTimeParamStart + 1;
const size_t parameters_end = command_end_position - parameters_length;

Expand Down
3 changes: 2 additions & 1 deletion source/common/api/os_sys_calls_impl.cc
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
#include "common/api/os_sys_calls_impl.h"

#include <errno.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <unistd.h>

#include <cerrno>

namespace Envoy {
namespace Api {

Expand Down
2 changes: 1 addition & 1 deletion source/common/api/os_sys_calls_impl_hot_restart.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "common/api/os_sys_calls_impl_hot_restart.h"

#include <errno.h>
#include <cerrno>

namespace Envoy {
namespace Api {
Expand Down
3 changes: 2 additions & 1 deletion source/common/api/os_sys_calls_impl_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@

#include "common/api/os_sys_calls_impl_linux.h"

#include <errno.h>
#include <sched.h>

#include <cerrno>

namespace Envoy {
namespace Api {

Expand Down
2 changes: 1 addition & 1 deletion source/common/buffer/buffer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ class OwnedImpl : public LibEventInstance {

// LibEventInstance
Event::Libevent::BufferPtr& buffer() override { return buffer_; }
virtual void postProcess() override;
void postProcess() override;

/**
* Create a new slice at the end of the buffer, and copy the supplied content into it.
Expand Down
8 changes: 4 additions & 4 deletions source/common/buffer/zero_copy_input_stream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ class ZeroCopyInputStreamImpl : public virtual Protobuf::io::ZeroCopyInputStream
// Note Next() will return true with no data until more data is available if the stream is not
// finished. It is the caller's responsibility to finish the stream or wrap with
// LimitingInputStream before passing to protobuf code to avoid a spin loop.
virtual bool Next(const void** data, int* size) override;
virtual void BackUp(int count) override;
virtual bool Skip(int count) override; // Not implemented
virtual ProtobufTypes::Int64 ByteCount() const override { return byte_count_; }
bool Next(const void** data, int* size) override;
void BackUp(int count) override;
bool Skip(int count) override; // Not implemented
ProtobufTypes::Int64 ByteCount() const override { return byte_count_; }

protected:
Buffer::InstancePtr buffer_;
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/assert.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ActionRegistrationImpl : public ActionRegistration {

std::function<void()> ActionRegistrationImpl::debug_assertion_failure_record_action_;

ActionRegistrationPtr setDebugAssertionFailureRecordAction(std::function<void()> action) {
ActionRegistrationPtr setDebugAssertionFailureRecordAction(const std::function<void()>& action) {
return std::make_unique<ActionRegistrationImpl>(action);
}

Expand Down
2 changes: 1 addition & 1 deletion source/common/common/assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ using ActionRegistrationPtr = std::unique_ptr<ActionRegistration>;
* @param action The action to take when an assertion fails.
* @return A registration object. The registration is removed when the object is destructed.
*/
ActionRegistrationPtr setDebugAssertionFailureRecordAction(std::function<void()> action);
ActionRegistrationPtr setDebugAssertionFailureRecordAction(const std::function<void()>& action);

/**
* Invokes the action set by setDebugAssertionFailureRecordAction, or does nothing if
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/empty_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
#include <string>

namespace Envoy {
static const std::string EMPTY_STRING = "";
static const std::string EMPTY_STRING;
} // namespace Envoy
6 changes: 3 additions & 3 deletions source/common/common/linked_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ template <class T> class LinkedObject {
}

protected:
LinkedObject() : inserted_(false) {}
LinkedObject() = default;

private:
typename ListType::iterator entry_;
bool inserted_; // iterators do not have any "invalid" value so we need this boolean for sanity
// checking.
bool inserted_{false}; // iterators do not have any "invalid" value so we need this boolean for
// sanity checking.
};
} // namespace Envoy
4 changes: 2 additions & 2 deletions source/common/common/logger.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ class Logger {
* but the method to log at err level is called LOGGER.error not LOGGER.err. All other level are
* fine spdlog::info corresponds to LOGGER.info method.
*/
typedef enum {
using levels = enum {
trace = spdlog::level::trace,
debug = spdlog::level::debug,
info = spdlog::level::info,
warn = spdlog::level::warn,
error = spdlog::level::err,
critical = spdlog::level::critical,
off = spdlog::level::off
} levels;
};

spdlog::string_view_t levelString() const {
return spdlog::level::level_string_views[logger_->level()];
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/matchers.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class ListMatcher : public ValueMatcher {
public:
ListMatcher(const envoy::type::matcher::ListMatcher& matcher);

bool match(const ProtobufWkt::Value& value) const;
bool match(const ProtobufWkt::Value& value) const override;

private:
const envoy::type::matcher::ListMatcher matcher_;
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/non_copyable.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Envoy {
*/
class NonCopyable {
protected:
NonCopyable() {}
NonCopyable() = default;

private:
NonCopyable(const NonCopyable&);
Expand Down
4 changes: 2 additions & 2 deletions source/common/common/perf_annotation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void PerfOperation::record(absl::string_view category, absl::string_view descrip
// The ctor is explicitly declared private to encourage clients to use getOrCreate(), at
// least for now. Given that it's declared it must be instantiated. It's not inlined
// because the constructor is non-trivial due to the contained unordered_map.
PerfAnnotationContext::PerfAnnotationContext() {}
PerfAnnotationContext::PerfAnnotationContext() = default;

void PerfAnnotationContext::record(std::chrono::nanoseconds duration, absl::string_view category,
absl::string_view description) {
Expand Down Expand Up @@ -146,7 +146,7 @@ void PerfAnnotationContext::clear() {
}

PerfAnnotationContext* PerfAnnotationContext::getOrCreate() {
static PerfAnnotationContext* context = new PerfAnnotationContext();
static auto* context = new PerfAnnotationContext();
return context;
}

Expand Down
3 changes: 1 addition & 2 deletions source/common/common/scalar_to_byte_vector.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#pragma once

#include <inttypes.h>

#include <cinttypes>
#include <vector>

namespace Envoy {
Expand Down
2 changes: 1 addition & 1 deletion source/common/common/stack_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <alloca.h>
#endif

#include <stddef.h>
#include <cstddef>

#include "common/common/assert.h"

Expand Down
2 changes: 1 addition & 1 deletion source/common/common/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ class InlineString : public InlineStorage {
/**
* @return a string_view into the InlineString.
*/
absl::string_view toStringView() const { return absl::string_view(data_, size_); }
absl::string_view toStringView() const { return {data_, size_}; }

/**
* @return the number of bytes in the string
Expand Down
2 changes: 1 addition & 1 deletion source/common/config/metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const ProtobufWkt::Value& Metadata::metadataValue(const envoy::api::v2::core::Me
const ProtobufWkt::Struct* data_struct = &(filter_it->second);
const ProtobufWkt::Value* val = nullptr;
// go through path to select sub entries
for (const auto p : path) {
for (const auto& p : path) {
if (nullptr == data_struct) { // sub entry not found
return ProtobufWkt::Value::default_instance();
}
Expand Down
36 changes: 18 additions & 18 deletions source/common/config/well_known_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,62 +44,62 @@ TagNameValues::TagNameValues() {

// mongo.[<stat_prefix>.]collection.[<collection>.]callsite.(<callsite>.)query.<base_stat>
addRegex(MONGO_CALLSITE,
"^mongo(?=\\.).*?\\.collection(?=\\.).*?\\.callsite\\.((.*?)\\.).*?query.\\w+?$",
R"(^mongo(?=\.).*?\.collection(?=\.).*?\.callsite\.((.*?)\.).*?query.\w+?$)",
".collection.");

// http.[<stat_prefix>.]dynamodb.table.(<table_name>.) or
// http.[<stat_prefix>.]dynamodb.error.(<table_name>.)*
addRegex(DYNAMO_TABLE, "^http(?=\\.).*?\\.dynamodb.(?:table|error)\\.((.*?)\\.)", ".dynamodb.");
addRegex(DYNAMO_TABLE, R"(^http(?=\.).*?\.dynamodb.(?:table|error)\.((.*?)\.))", ".dynamodb.");

// mongo.[<stat_prefix>.]collection.(<collection>.)query.<base_stat>
addRegex(MONGO_COLLECTION, "^mongo(?=\\.).*?\\.collection\\.((.*?)\\.).*?query.\\w+?$",
addRegex(MONGO_COLLECTION, R"(^mongo(?=\.).*?\.collection\.((.*?)\.).*?query.\w+?$)",
".collection.");

// mongo.[<stat_prefix>.]cmd.(<cmd>.)<base_stat>
addRegex(MONGO_CMD, "^mongo(?=\\.).*?\\.cmd\\.((.*?)\\.)\\w+?$", ".cmd.");
addRegex(MONGO_CMD, R"(^mongo(?=\.).*?\.cmd\.((.*?)\.)\w+?$)", ".cmd.");

// cluster.[<route_target_cluster>.]grpc.[<grpc_service>.](<grpc_method>.)<base_stat>
addRegex(GRPC_BRIDGE_METHOD, "^cluster(?=\\.).*?\\.grpc(?=\\.).*\\.((.*?)\\.)\\w+?$", ".grpc.");
addRegex(GRPC_BRIDGE_METHOD, R"(^cluster(?=\.).*?\.grpc(?=\.).*\.((.*?)\.)\w+?$)", ".grpc.");

// http.[<stat_prefix>.]user_agent.(<user_agent>.)<base_stat>
addRegex(HTTP_USER_AGENT, "^http(?=\\.).*?\\.user_agent\\.((.*?)\\.)\\w+?$", ".user_agent.");
addRegex(HTTP_USER_AGENT, R"(^http(?=\.).*?\.user_agent\.((.*?)\.)\w+?$)", ".user_agent.");

// vhost.[<virtual host name>.]vcluster.(<virtual_cluster_name>.)<base_stat>
addRegex(VIRTUAL_CLUSTER, "^vhost(?=\\.).*?\\.vcluster\\.((.*?)\\.)\\w+?$", ".vcluster.");
addRegex(VIRTUAL_CLUSTER, R"(^vhost(?=\.).*?\.vcluster\.((.*?)\.)\w+?$)", ".vcluster.");

// http.[<stat_prefix>.]fault.(<downstream_cluster>.)<base_stat>
addRegex(FAULT_DOWNSTREAM_CLUSTER, "^http(?=\\.).*?\\.fault\\.((.*?)\\.)\\w+?$", ".fault.");
addRegex(FAULT_DOWNSTREAM_CLUSTER, R"(^http(?=\.).*?\.fault\.((.*?)\.)\w+?$)", ".fault.");

// listener.[<address>.]ssl.cipher.(<cipher>)
addRegex(SSL_CIPHER, "^listener(?=\\.).*?\\.ssl\\.cipher(\\.(.*?))$");
addRegex(SSL_CIPHER, R"(^listener(?=\.).*?\.ssl\.cipher(\.(.*?))$)");

// cluster.[<cluster_name>.]ssl.ciphers.(<cipher>)
addRegex(SSL_CIPHER_SUITE, "^cluster(?=\\.).*?\\.ssl\\.ciphers(\\.(.*?))$", ".ssl.ciphers.");
addRegex(SSL_CIPHER_SUITE, R"(^cluster(?=\.).*?\.ssl\.ciphers(\.(.*?))$)", ".ssl.ciphers.");

// cluster.[<route_target_cluster>.]grpc.(<grpc_service>.)*
addRegex(GRPC_BRIDGE_SERVICE, "^cluster(?=\\.).*?\\.grpc\\.((.*?)\\.)", ".grpc.");
addRegex(GRPC_BRIDGE_SERVICE, R"(^cluster(?=\.).*?\.grpc\.((.*?)\.))", ".grpc.");

// tcp.(<stat_prefix>.)<base_stat>
addRegex(TCP_PREFIX, "^tcp\\.((.*?)\\.)\\w+?$");
addRegex(TCP_PREFIX, R"(^tcp\.((.*?)\.)\w+?$)");

// auth.clientssl.(<stat_prefix>.)<base_stat>
addRegex(CLIENTSSL_PREFIX, "^auth\\.clientssl\\.((.*?)\\.)\\w+?$");
addRegex(CLIENTSSL_PREFIX, R"(^auth\.clientssl\.((.*?)\.)\w+?$)");

// ratelimit.(<stat_prefix>.)<base_stat>
addRegex(RATELIMIT_PREFIX, "^ratelimit\\.((.*?)\\.)\\w+?$");
addRegex(RATELIMIT_PREFIX, R"(^ratelimit\.((.*?)\.)\w+?$)");

// cluster.(<cluster_name>.)*
addRegex(CLUSTER_NAME, "^cluster\\.((.*?)\\.)");

// listener.[<address>.]http.(<stat_prefix>.)*
addRegex(HTTP_CONN_MANAGER_PREFIX, "^listener(?=\\.).*?\\.http\\.((.*?)\\.)", ".http.");
addRegex(HTTP_CONN_MANAGER_PREFIX, R"(^listener(?=\.).*?\.http\.((.*?)\.))", ".http.");

// http.(<stat_prefix>.)*
addRegex(HTTP_CONN_MANAGER_PREFIX, "^http\\.((.*?)\\.)");

// listener.(<address>.)*
addRegex(LISTENER_ADDRESS,
"^listener\\.(((?:[_.[:digit:]]*|[_\\[\\]aAbBcCdDeEfF[:digit:]]*))\\.)");
R"(^listener\.(((?:[_.[:digit:]]*|[_\[\]aAbBcCdDeEfF[:digit:]]*))\.))");

// vhost.(<virtual host name>.)*
addRegex(VIRTUAL_HOST, "^vhost\\.((.*?)\\.)");
Expand All @@ -108,10 +108,10 @@ TagNameValues::TagNameValues() {
addRegex(MONGO_PREFIX, "^mongo\\.((.*?)\\.)");

// http.[<stat_prefix>.]rds.(<route_config_name>.)<base_stat>
addRegex(RDS_ROUTE_CONFIG, "^http(?=\\.).*?\\.rds\\.((.*?)\\.)\\w+?$", ".rds.");
addRegex(RDS_ROUTE_CONFIG, R"(^http(?=\.).*?\.rds\.((.*?)\.)\w+?$)", ".rds.");

// listener_manager.(worker_<id>.)*
addRegex(WORKER_ID, "^listener_manager\\.((worker_\\d+)\\.)", "listener_manager.worker_");
addRegex(WORKER_ID, R"(^listener_manager\.((worker_\d+)\.))", "listener_manager.worker_");
}

void TagNameValues::addRegex(const std::string& name, const std::string& regex,
Expand Down
Loading