Delint arguments.cc and arguments.hh

diff --git a/pdns/arguments.cc b/pdns/arguments.cc
index 70f66a93eb8447f43695dcffcf7765c0c8b3ee45..32811e51171bc1590e28dd015d42104c97f68118 100644 (file)
--- a/pdns/arguments.cc
+++ b/pdns/arguments.cc
@@ -35,12 +35,12 @@
 #include <unistd.h>
 #include <climits>
 
-const ArgvMap::param_t::const_iterator ArgvMap::begin()
+ArgvMap::param_t::const_iterator ArgvMap::begin()
 {
   return d_params.begin();
 }
 
-const ArgvMap::param_t::const_iterator ArgvMap::end()
+ArgvMap::param_t::const_iterator ArgvMap::end()
 {
   return d_params.end();
 }
@@ -52,15 +52,18 @@ string& ArgvMap::set(const string& var)
 
 void ArgvMap::setDefault(const string& var, const string& value)
 {
-  if (!defaultmap.count(var))
+  if (defaultmap.count(var) == 0) {
     defaultmap.insert(pair<string, string>(var, value));
+  }
 }
 
 void ArgvMap::setDefaults()
 {
-  for (const auto& i : d_params)
-    if (!defaultmap.count(i.first))
-      defaultmap.insert(i);
+  for (const auto& param : d_params) {
+    if (defaultmap.count(param.first) == 0) {
+      defaultmap.insert(param);
+    }
+  }
 }
 
 bool ArgvMap::mustDo(const string& var)
@@ -71,8 +74,10 @@ bool ArgvMap::mustDo(const string& var)
 vector<string> ArgvMap::list()
 {
   vector<string> ret;
-  for (const auto& i : d_params)
-    ret.push_back(i.first);
+  ret.reserve(d_params.size());
+  for (const auto& param : d_params) {
+    ret.push_back(param.first);
+  }
   return ret;
 }
 
@@ -111,46 +116,43 @@ bool ArgvMap::contains(const string& var, const string& val)
   vector<string> parts;
 
   stringtok(parts, param->second, ", \t");
-  for (const auto& part : parts) {
-    if (part == val) {
-      return true;
-    }
-  }
-
-  return false;
+  return std::any_of(parts.begin(), parts.end(), [&](const std::string& str) { return str == val; });
 }
 
 string ArgvMap::helpstring(string prefix)
 {
-  if (prefix == "no")
+  if (prefix == "no") {
     prefix = "";
+  }
 
   string help;
 
-  for (const auto& i : helpmap) {
-    if (!prefix.empty() && i.first.find(prefix) != 0) // only print items with prefix
+  for (const auto& helpitem : helpmap) {
+    if (!prefix.empty() && helpitem.first.find(prefix) != 0) { // only print items with prefix
       continue;
+    }
 
     help += "  --";
-    help += i.first;
+    help += helpitem.first;
 
-    string type = d_typeMap[i.first];
+    string type = d_typeMap[helpitem.first];
 
-    if (type == "Parameter")
+    if (type == "Parameter") {
       help += "=...";
+    }
     else if (type == "Switch") {
-      help += " | --" + i.first + "=yes";
-      help += " | --" + i.first + "=no";
+      help += " | --" + helpitem.first + "=yes";
+      help += " | --" + helpitem.first + "=no";
     }
 
     help += "\n\t";
-    help += i.second;
+    help += helpitem.second;
     help += "\n";
   }
   return help;
 }
 
-const string ArgvMap::formatOne(bool running, bool full, const string& var, const string& help, const string& theDefault, const string& current)
+string ArgvMap::formatOne(bool running, bool full, const string& var, const string& help, const string& theDefault, const string& current)
 {
   string out;
 
@@ -190,30 +192,34 @@ string ArgvMap::configstring(bool running, bool full)
 {
   string help;
 
-  if (running)
+  if (running) {
     help = "# Autogenerated configuration file based on running instance (" + nowTime() + ")\n\n";
-  else
+  }
+  else {
     help = "# Autogenerated configuration file template\n\n";
+  }
 
   // Affects parsing, should come first.
   help += formatOne(running, full, "ignore-unknown-settings", helpmap["ignore-unknown-settings"], defaultmap["ignore-unknown-settings"], d_params["ignore-unknown-settings"]);
 
-  for (const auto& i : helpmap) {
-    if (d_typeMap[i.first] == "Command")
+  for (const auto& helpietm : helpmap) {
+    if (d_typeMap[helpietm.first] == "Command") {
       continue;
-    if (i.first == "ignore-unknown-settings")
+    }
+    if (helpietm.first == "ignore-unknown-settings") {
       continue;
+    }
 
-    if (!defaultmap.count(i.first)) {
-      throw ArgException(string("Default for setting '") + i.first + "' not set");
+    if (defaultmap.count(helpietm.first) == 0) {
+      throw ArgException(string("Default for setting '") + helpietm.first + "' not set");
     }
 
-    help += formatOne(running, full, i.first, i.second, defaultmap[i.first], d_params[i.first]);
+    help += formatOne(running, full, helpietm.first, helpietm.second, defaultmap[helpietm.first], d_params[helpietm.first]);
   }
 
   if (running) {
-    for (const auto& i : d_unknownParams) {
-      help += formatOne(running, full, i.first, "unknown setting", "", i.second);
+    for (const auto& unknown : d_unknownParams) {
+      help += formatOne(running, full, unknown.first, "unknown setting", "", unknown.second);
     }
   }
 
@@ -222,44 +228,45 @@ string ArgvMap::configstring(bool running, bool full)
 
 const string& ArgvMap::operator[](const string& arg)
 {
-  if (!parmIsset(arg))
+  if (!parmIsset(arg)) {
     throw ArgException(string("Undefined but needed argument: '") + arg + "'");
+  }
 
   return d_params[arg];
 }
 
 mode_t ArgvMap::asMode(const string& arg)
 {
-  mode_t mode;
-  const char* cptr_orig;
-  char* cptr_ret = nullptr;
-
-  if (!parmIsset(arg))
+  if (!parmIsset(arg)) {
     throw ArgException(string("Undefined but needed argument: '") + arg + "'");
+  }
 
-  cptr_orig = d_params[arg].c_str();
-  mode = static_cast<mode_t>(strtol(cptr_orig, &cptr_ret, 8));
-  if (mode == 0 && cptr_ret == cptr_orig)
+  const auto* const cptr_orig = d_params[arg].c_str();
+  char* cptr_ret = nullptr;
+
+  auto mode = static_cast<mode_t>(strtol(cptr_orig, &cptr_ret, 8));
+  if (mode == 0 && cptr_ret == cptr_orig) {
     throw ArgException("'" + arg + string("' contains invalid octal mode"));
+  }
   return mode;
 }
 
 gid_t ArgvMap::asGid(const string& arg)
 {
-  gid_t gid;
-  const char* cptr_orig;
-  char* cptr_ret = nullptr;
-
-  if (!parmIsset(arg))
+  if (!parmIsset(arg)) {
     throw ArgException(string("Undefined but needed argument: '") + arg + "'");
+  }
 
-  cptr_orig = d_params[arg].c_str();
-  gid = static_cast<gid_t>(strtol(cptr_orig, &cptr_ret, 0));
+  const auto* cptr_orig = d_params[arg].c_str();
+  char* cptr_ret = nullptr;
+  auto gid = static_cast<gid_t>(strtol(cptr_orig, &cptr_ret, 0));
   if (gid == 0 && cptr_ret == cptr_orig) {
     // try to resolve
-    struct group* group = getgrnam(d_params[arg].c_str());
-    if (group == nullptr)
+
+    struct group* group = getgrnam(d_params[arg].c_str()); // NOLINT: called before going multi-threaded
+    if (group == nullptr) {
       throw ArgException("'" + arg + string("' contains invalid group"));
+    }
     gid = group->gr_gid;
   }
   return gid;
@@ -267,20 +274,20 @@ gid_t ArgvMap::asGid(const string& arg)
 
 uid_t ArgvMap::asUid(const string& arg)
 {
-  uid_t uid;
-  const char* cptr_orig;
-  char* cptr_ret = nullptr;
-
-  if (!parmIsset(arg))
+  if (!parmIsset(arg)) {
     throw ArgException(string("Undefined but needed argument: '") + arg + "'");
+  }
+
+  const auto* cptr_orig = d_params[arg].c_str();
+  char* cptr_ret = nullptr;
 
-  cptr_orig = d_params[arg].c_str();
-  uid = static_cast<uid_t>(strtol(cptr_orig, &cptr_ret, 0));
+  auto uid = static_cast<uid_t>(strtol(cptr_orig, &cptr_ret, 0));
   if (uid == 0 && cptr_ret == cptr_orig) {
     // try to resolve
-    struct passwd* pwent = getpwnam(d_params[arg].c_str());
-    if (pwent == nullptr)
+    struct passwd* pwent = getpwnam(d_params[arg].c_str()); // NOLINT: called before going multi-threaded
+    if (pwent == nullptr) {
       throw ArgException("'" + arg + string("' contains invalid group"));
+    }
     uid = pwent->pw_uid;
   }
   return uid;
@@ -288,49 +295,50 @@ uid_t ArgvMap::asUid(const string& arg)
 
 int ArgvMap::asNum(const string& arg, int def)
 {
-  int retval;
-  const char* cptr_orig;
-  char* cptr_ret = nullptr;
-
-  if (!parmIsset(arg))
+  if (!parmIsset(arg)) {
     throw ArgException(string("Undefined but needed argument: '") + arg + "'");
+  }
 
   // use default for empty values
-  if (d_params[arg].empty())
+  if (d_params[arg].empty()) {
     return def;
+  }
 
-  cptr_orig = d_params[arg].c_str();
-  retval = static_cast<int>(strtol(cptr_orig, &cptr_ret, 0));
-  if (!retval && cptr_ret == cptr_orig)
+  const auto* cptr_orig = d_params[arg].c_str();
+  char* cptr_ret = nullptr;
+  auto retval = static_cast<int>(strtol(cptr_orig, &cptr_ret, 0));
+  if (retval == 0 && cptr_ret == cptr_orig) {
     throw ArgException("'" + arg + "' value '" + string(cptr_orig) + string("' is not a valid number"));
+  }
 
   return retval;
 }
 
 bool ArgvMap::isEmpty(const string& arg)
 {
-  if (!parmIsset(arg))
+  if (!parmIsset(arg)) {
     return true;
+  }
   return d_params[arg].empty();
 }
 
 double ArgvMap::asDouble(const string& arg)
 {
-  double retval;
-  const char* cptr_orig;
-  char* cptr_ret = nullptr;
-
-  if (!parmIsset(arg))
+  if (!parmIsset(arg)) {
     throw ArgException(string("Undefined but needed argument: '") + arg + "'");
+  }
 
-  if (d_params[arg].empty())
+  if (d_params[arg].empty()) {
     return 0.0;
+  }
 
-  cptr_orig = d_params[arg].c_str();
-  retval = strtod(cptr_orig, &cptr_ret);
+  const auto* cptr_orig = d_params[arg].c_str();
+  char* cptr_ret = nullptr;
+  auto retval = strtod(cptr_orig, &cptr_ret);
 
-  if (retval == 0 && cptr_ret == cptr_orig)
+  if (retval == 0 && cptr_ret == cptr_orig) {
     throw ArgException("'" + arg + string("' is not valid double"));
+  }
 
   return retval;
 }
@@ -369,8 +377,9 @@ void ArgvMap::warnIfDeprecated(const string& var)
 
 void ArgvMap::parseOne(const string& arg, const string& parseOnly, bool lax)
 {
-  string var, val;
-  string::size_type pos;
+  string var;
+  string val;
+  string::size_type pos = 0;
   bool incremental = false;
 
   if (arg.find("--") == 0 && (pos = arg.find("+=")) != string::npos) // this is a --port+=25 case
@@ -393,27 +402,31 @@ void ArgvMap::parseOne(const string& arg, const string& parseOnly, bool lax)
     var = arg.substr(1);
     val = "";
   }
-  else // command
+  else { // command
     d_cmds.push_back(arg);
+  }
 
   boost::trim(var);
 
-  if (var != "" && (parseOnly.empty() || var == parseOnly)) {
+  if (!var.empty() && (parseOnly.empty() || var == parseOnly)) {
     if (!lax) {
       warnIfDeprecated(var);
     }
     pos = val.find_first_not_of(" \t"); // strip leading whitespace
-    if (pos && pos != string::npos)
+    if (pos != 0 && pos != string::npos) {
       val = val.substr(pos);
+    }
     if (parmIsset(var)) {
       if (incremental) {
         if (d_params[var].empty()) {
-          if (!d_cleared.count(var))
+          if (d_cleared.count(var) == 0) {
             throw ArgException("Incremental setting '" + var + "' without a parent");
+          }
           d_params[var] = val;
         }
-        else
+        else {
           d_params[var] += ", " + val;
+        }
       }
       else {
         d_params[var] = val;
@@ -447,17 +460,18 @@ void ArgvMap::parse(int& argc, char** argv, bool lax)
 {
   d_cmds.clear();
   d_cleared.clear();
-  for (int n = 1; n < argc; n++) {
-    parseOne(argv[n], "", lax);
+  for (int i = 1; i < argc; i++) {
+    parseOne(argv[i], "", lax); // NOLINT: Posix argument parsing
   }
 }
 
 void ArgvMap::preParse(int& argc, char** argv, const string& arg)
 {
-  for (int n = 1; n < argc; n++) {
-    string varval = argv[n];
-    if (varval.find("--" + arg) == 0)
-      parseOne(argv[n]);
+  for (int i = 1; i < argc; i++) {
+    string varval = argv[i]; // NOLINT: Posix argument parsing
+    if (varval.find("--" + arg) == 0) {
+      parseOne(argv[i]); // NOLINT:  Posix argument parsing
+    }
   }
 }
 
@@ -552,28 +566,32 @@ bool ArgvMap::file(const char* fname, bool lax, bool included)
 void ArgvMap::gatherIncludes(std::vector<std::string>& extraConfigs)
 {
   extraConfigs.clear();
-  if (d_params["include-dir"].empty())
+  if (d_params["include-dir"].empty()) {
     return; // nothing to do
+  }
 
-  DIR* dir;
-  if (!(dir = opendir(d_params["include-dir"].c_str()))) {
+  DIR* dir = nullptr;
+  if ((dir = opendir(d_params["include-dir"].c_str())) == nullptr) {
     int err = errno;
-    string msg = d_params["include-dir"] + " is not accessible: " + strerror(err);
+    string msg = d_params["include-dir"] + " is not accessible: " + stringerror(err);
     SLOG(g_log << Logger::Error << msg << std::endl,
          d_log->error(Logr::Error, err, "Directory is not accessible", "name", Logging::Loggable(d_params["include-dir"])));
     throw ArgException(msg);
   }
 
-  struct dirent* ent;
-  while ((ent = readdir(dir)) != nullptr) {
-    if (ent->d_name[0] == '.')
+  struct dirent* ent = nullptr;
+  while ((ent = readdir(dir)) != nullptr) { // NOLINT(concurrency-mt-unsafe): see Linux man page
+    if (ent->d_name[0] == '.') {
       continue; // skip any dots
+    }
     if (boost::ends_with(ent->d_name, ".conf")) {
       // build name
-      string name = d_params["include-dir"] + "/" + ent->d_name; // FIXME: Use some path separator
+      string name = d_params["include-dir"] + "/" + ent->d_name; // NOLINT: Posix API
       // ensure it's readable file
-      struct stat st;
-      if (stat(name.c_str(), &st) || !S_ISREG(st.st_mode)) {
+      struct stat statInfo
+      {
+      };
+      if (stat(name.c_str(), &statInfo) != 0 || !S_ISREG(statInfo.st_mode)) {
         string msg = name + " is not a regular file";
         SLOG(g_log << Logger::Error << msg << std::endl,
              d_log->info(Logr::Error, "Unable to open non-regular file", "name", Logging::Loggable(name)));

diff --git a/pdns/arguments.hh b/pdns/arguments.hh
index 402608220a300f4fdef18e0c59867d49c8b554a2..fa5bfcf35547aaff7ebe09b443ec0728456e85b3 100644 (file)
--- a/pdns/arguments.hh
+++ b/pdns/arguments.hh
@@ -35,7 +35,7 @@
 #include "namespaces.hh"
 #include "logging.hh"
 
-typedef PDNSException ArgException;
+using ArgException = PDNSException;
 
 /** This class helps parsing argc and argv into a map of parameters. We have 3 kinds of formats:
 
@@ -90,30 +90,31 @@ public:
     return file(fname, true);
   }
   bool parseFile(const char* fname, const string& arg, bool lax); //<! parse one line
-  typedef map<string, string> param_t; //!< use this if you need to know the content of the map
   bool parmIsset(const string& var); //!< Checks if a parameter is set to *a* value
   bool mustDo(const string& var); //!< if a switch is given, if we must do something (--help)
-  int asNum(const string& var, int def = 0); //!< return a variable value as a number or the default if the variable is empty
-  mode_t asMode(const string& var); //!< return value interpreted as octal number
-  uid_t asUid(const string& var); //!< return user id, resolves if necessary
-  gid_t asGid(const string& var); //!< return group id, resolves if necessary
-  double asDouble(const string& var); //!< return a variable value as a number
+  int asNum(const string& arg, int def = 0); //!< return a variable value as a number or the default if the variable is empty
+  mode_t asMode(const string& arg); //!< return value interpreted as octal number
+  uid_t asUid(const string& arg); //!< return user id, resolves if necessary
+  gid_t asGid(const string& arg); //!< return group id, resolves if necessary
+  double asDouble(const string& arg); //!< return a variable value as a number
   string& set(const string&); //!< Gives a writable reference and allocates space for it
   string& set(const string&, const string&); //!< Does the same but also allows one to specify a help message
   void setCmd(const string&, const string&); //!< Add a command flag
   string& setSwitch(const string&, const string&); //!< Add a switch flag
   string helpstring(string prefix = ""); //!< generates the --help
-  string configstring(bool current, bool full); //!< generates the --config
+  string configstring(bool running, bool full); //!< generates the --config
   bool contains(const string& var, const string& val);
-  bool isEmpty(const string& var); //!< checks if variable has value
+  bool isEmpty(const string& arg); //!< checks if variable has value
   void setDefault(const string& var, const string& value);
   void setDefaults();
 
   vector<string> list();
   string getHelp(const string& item);
 
-  const param_t::const_iterator begin(); //!< iterator semantics
-  const param_t::const_iterator end(); //!< iterator semantics
+  using param_t = map<string, string>; //!< use this if you need to know the content of the map
+
+  param_t::const_iterator begin(); //!< iterator semantics
+  param_t::const_iterator end(); //!< iterator semantics
   const string& operator[](const string&); //!< iterator semantics
   const vector<string>& getCommands();
   void gatherIncludes(std::vector<std::string>& extraConfigs);
@@ -125,8 +126,8 @@ public:
 #endif
 private:
   void warnIfDeprecated(const string& var);
-  void parseOne(const string& unparsed, const string& parseOnly = "", bool lax = false);
-  const string formatOne(bool running, bool full, const string& var, const string& help, const string& theDefault, const string& value);
+  void parseOne(const string& arg, const string& parseOnly = "", bool lax = false);
+  static string formatOne(bool running, bool full, const string& var, const string& help, const string& theDefault, const string& current);
   map<string, string> d_params;
   map<string, string> d_unknownParams;
   map<string, string> helpmap;