From 1ad23f6b9e9c1c2553e4237436e7a5076aedf577 Mon Sep 17 00:00:00 2001
From: Refael Ackermann <refack@gmail.com>
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 <joyeec9h3@gmail.com>
---
 src/node_config.cc         |  2 +-
 src/node_options-inl.h     | 10 +++++-----
 src/node_options.cc        |  9 +++++----
 src/node_options.h         |  2 +-
 src/node_process_object.cc |  4 ++--
 src/node_worker.cc         |  4 ++--
 6 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/node_config.cc b/src/node_config.cc
index 2b8ff8abff94d7..7ba55cd7efb2f6 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 4387ec54bc34cc..9c8d24e29e9c7d 100644
--- a/src/node_options-inl.h
+++ b/src/node_options-inl.h
@@ -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));
 }
 
diff --git a/src/node_options.cc b/src/node_options.cc
index 98da213bff1ba4..f384957db5bef3 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -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);
   }
 };
 
@@ -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(
@@ -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) {
@@ -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.");
diff --git a/src/node_options.h b/src/node_options.h
index 072e001fb712ea..5c9dbe77ab47ef 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -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
diff --git a/src/node_process_object.cc b/src/node_process_object.cc
index e4d9d97c681bdd..35518780919f06 100644
--- a/src/node_process_object.cc
+++ b/src/node_process_object.cc
@@ -1,5 +1,3 @@
-#include <climits>  // 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 <climits>  // PATH_MAX
+
 namespace node {
 using v8::Context;
 using v8::DEFAULT;
diff --git a/src/node_worker.cc b/src/node_worker.cc
index 4bd82e3f385c4a..a4088b4260357d 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -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;
       }
     }