Skip to content

Commit

Permalink
src: fix warnings around node_options
Browse files Browse the repository at this point in the history
* 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: nodejs#26280
Fixes: nodejs#25593
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
refack committed Mar 14, 2019
1 parent 4c02851 commit 1ad23f6
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/node_config.cc
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions src/node_options-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,15 +213,15 @@ auto OptionsParser<Options>::Convert(
template <typename Options>
template <typename ChildOptions>
void OptionsParser<Options>::Insert(
const OptionsParser<ChildOptions>* child_options_parser,
const OptionsParser<ChildOptions>& 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));
}

Expand Down
9 changes: 5 additions & 4 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class EnvironmentOptionsParser : public OptionsParser<EnvironmentOptions> {
EnvironmentOptionsParser();
explicit EnvironmentOptionsParser(const DebugOptionsParser& dop)
: EnvironmentOptionsParser() {
Insert(&dop, &EnvironmentOptions::get_debug_options);
Insert(dop, &EnvironmentOptions::get_debug_options);
}
};

Expand Down Expand Up @@ -386,7 +386,7 @@ PerIsolateOptionsParser::PerIsolateOptionsParser(
kAllowedInEnvironment);
#endif // NODE_REPORT

Insert(&eop, &PerIsolateOptions::get_per_env_options);
Insert(eop, &PerIsolateOptions::get_per_env_options);
}

PerProcessOptionsParser::PerProcessOptionsParser(
Expand Down Expand Up @@ -496,7 +496,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) {
Expand All @@ -510,7 +510,8 @@ inline int ParseAndValidatePort(const std::string& port,
std::vector<std::string>* 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.");
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ class OptionsParser {
// a method that yields the target options type from this parser's options
// type.
template <typename ChildOptions>
void Insert(const OptionsParser<ChildOptions>* child_options_parser,
void Insert(const OptionsParser<ChildOptions>& child_options_parser,
ChildOptions* (Options::* get_child)());

// Parse a sequence of options into an options struct, a list of
Expand Down
4 changes: 2 additions & 2 deletions src/node_process_object.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#include <climits> // PATH_MAX

#include "env-inl.h"
#include "node_internals.h"
#include "node_options-inl.h"
Expand All @@ -8,6 +6,8 @@
#include "node_revert.h"
#include "util-inl.h"

#include <climits> // PATH_MAX

namespace node {
using v8::Context;
using v8::DEFAULT;
Expand Down
4 changes: 2 additions & 2 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -484,13 +484,13 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
// The first argument is program name.
invalid_args.erase(invalid_args.begin());
if (errors.size() > 0 || invalid_args.size() > 0) {
v8::Local<v8::Value> value =
v8::Local<v8::Value> error =
ToV8Value(env->context(),
errors.size() > 0 ? errors : invalid_args)
.ToLocalChecked();
Local<String> 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;
}
}
Expand Down

0 comments on commit 1ad23f6

Please sign in to comment.