From 668ca360a00c5435b0a40e09fde57228808d8be2 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Mon, 25 Feb 2019 15:42:28 -0500 Subject: [PATCH] src: fix warnings around node_options * header explicit usage, order, and reduce use of `*-inl.h` * pointer -> const reference when possible * no variable recyclicng * `std::begin/end` prefered over `instance.begin/end` * `USE` for explicit unused resaults PR-URL: https://github.com/nodejs/node/pull/26280 Fixes: https://github.com/nodejs/node/issues/25593 Reviewed-By: Joyee Cheung --- src/node_config.cc | 2 +- src/node_options-inl.h | 10 +++++----- src/node_options.cc | 12 +++++++----- src/node_options.h | 2 +- src/node_process_object.cc | 4 ++-- src/node_worker.cc | 4 ++-- 6 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/node_config.cc b/src/node_config.cc index 7ecf70fdb6256a..6d11f14d78c353 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -1,7 +1,7 @@ #include "env-inl.h" #include "node.h" #include "node_i18n.h" -#include "node_options-inl.h" +#include "node_options.h" #include "util-inl.h" namespace node { diff --git a/src/node_options-inl.h b/src/node_options-inl.h index 052c847f7ebdc4..001bdecbb67361 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -213,15 +213,15 @@ auto OptionsParser::Convert( template template void OptionsParser::Insert( - const OptionsParser* child_options_parser, + const OptionsParser& child_options_parser, ChildOptions* (Options::* get_child)()) { - aliases_.insert(child_options_parser->aliases_.begin(), - child_options_parser->aliases_.end()); + aliases_.insert(std::begin(child_options_parser.aliases_), + std::end(child_options_parser.aliases_)); - for (const auto& pair : child_options_parser->options_) + for (const auto& pair : child_options_parser.options_) options_.emplace(pair.first, Convert(pair.second, get_child)); - for (const auto& pair : child_options_parser->implications_) + for (const auto& pair : child_options_parser.implications_) implications_.emplace(pair.first, Convert(pair.second, get_child)); } diff --git a/src/node_options.cc b/src/node_options.cc index 206c05797ed86f..1033b1f7e14c29 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -1,8 +1,9 @@ -#include #include "env-inl.h" #include "node_binding.h" #include "node_options-inl.h" +#include // strtoul, errno + using v8::Boolean; using v8::Context; using v8::FunctionCallbackInfo; @@ -112,7 +113,7 @@ class EnvironmentOptionsParser : public OptionsParser { EnvironmentOptionsParser(); explicit EnvironmentOptionsParser(const DebugOptionsParser& dop) : EnvironmentOptionsParser() { - Insert(&dop, &EnvironmentOptions::get_debug_options); + Insert(dop, &EnvironmentOptions::get_debug_options); } }; @@ -373,7 +374,7 @@ PerIsolateOptionsParser::PerIsolateOptionsParser( kAllowedInEnvironment); #endif // NODE_REPORT - Insert(&eop, &PerIsolateOptions::get_per_env_options); + Insert(eop, &PerIsolateOptions::get_per_env_options); } PerProcessOptionsParser::PerProcessOptionsParser( @@ -483,7 +484,7 @@ PerProcessOptionsParser::PerProcessOptionsParser( #endif #endif - Insert(&iop, &PerProcessOptions::get_per_isolate_options); + Insert(iop, &PerProcessOptions::get_per_isolate_options); } inline std::string RemoveBrackets(const std::string& host) { @@ -497,7 +498,8 @@ inline int ParseAndValidatePort(const std::string& port, std::vector* errors) { char* endptr; errno = 0; - const long result = strtol(port.c_str(), &endptr, 10); // NOLINT(runtime/int) + const unsigned long result = // NOLINT(runtime/int) + strtoul(port.c_str(), &endptr, 10); if (errno != 0 || *endptr != '\0'|| (result != 0 && result < 1024) || result > 65535) { errors->push_back(" must be 0 or in range 1024 to 65535."); diff --git a/src/node_options.h b/src/node_options.h index cc91d708ef284b..ccebe070ef3c37 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -300,7 +300,7 @@ class OptionsParser { // a method that yields the target options type from this parser's options // type. template - void Insert(const OptionsParser* child_options_parser, + void Insert(const OptionsParser& child_options_parser, ChildOptions* (Options::* get_child)()); // Parse a sequence of options into an options struct, a list of diff --git a/src/node_process_object.cc b/src/node_process_object.cc index a7af5e2d32b972..614c9a163f1dc4 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -1,5 +1,3 @@ -#include // PATH_MAX - #include "env-inl.h" #include "node_internals.h" #include "node_options-inl.h" @@ -8,6 +6,8 @@ #include "node_revert.h" #include "util-inl.h" +#include // PATH_MAX + namespace node { using v8::Context; using v8::DEFAULT; diff --git a/src/node_worker.cc b/src/node_worker.cc index e22503863d7db4..5ab4fad5d40114 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -487,13 +487,13 @@ void Worker::New(const FunctionCallbackInfo& args) { // The first argument is program name. invalid_args.erase(invalid_args.begin()); if (errors.size() > 0 || invalid_args.size() > 0) { - v8::Local value = + v8::Local error = ToV8Value(env->context(), errors.size() > 0 ? errors : invalid_args) .ToLocalChecked(); Local key = FIXED_ONE_BYTE_STRING(env->isolate(), "invalidExecArgv"); - args.This()->Set(env->context(), key, value).FromJust(); + USE(args.This()->Set(env->context(), key, error).FromJust()); return; } }