From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 1 Aug 2023 14:20:50 +0000 (+0200)
Subject: ixfrdist: Reduce the complexity of the main function
X-Git-Tag: rec-5.0.0-alpha1~62^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=48d1f24b87dfd95696c0db8311c81dc622717102;p=thirdparty%2Fpdns.git

ixfrdist: Reduce the complexity of the main function
---

diff --git a/pdns/ixfrdist.cc b/pdns/ixfrdist.cc
index d4d319927d..08528e5082 100644
--- a/pdns/ixfrdist.cc
+++ b/pdns/ixfrdist.cc
@@ -1179,12 +1179,26 @@ static bool parseAndCheckConfig(const string& configpath, YAML::Node& config) {
   return retval;
 }
 
-int main(int argc, char** argv) {
-  g_log.setLoglevel(Logger::Notice);
-  g_log.toConsole(Logger::Notice);
-  g_log.setPrefixed(true);
-  g_log.disableSyslog(true);
-  g_log.setTimestamps(false);
+struct IXFRDistConfiguration
+{
+  set<int> listeningSockets;
+  NetmaskGroup wsACL;
+  ComboAddress wsAddr;
+  std::string wsLogLevel{"normal"};
+  std::string workDir;
+  uint32_t axfrMaxRecords{0};
+  uint16_t keep{0};
+  uint16_t axfrTimeout{0};
+  uint16_t failedSOARetry{0};
+  uint16_t tcpInThreads{0};
+  uid_t uid{0};
+  gid_t gid{0};
+  bool done{false};
+};
+
+static std::optional<IXFRDistConfiguration> parseConfiguration(int argc, char** argv, FDMultiplexer& fdm)
+{
+  IXFRDistConfiguration configuration;
   po::variables_map g_vm;
   std::string configPath;
 
@@ -1203,23 +1217,23 @@ int main(int argc, char** argv) {
 
     if (g_vm.count("help") > 0) {
       usage(desc);
-      return EXIT_SUCCESS;
+      return configuration;
     }
 
     if (g_vm.count("version") > 0) {
       cout<<"ixfrdist "<<VERSION<<endl;
-      return EXIT_SUCCESS;
+      return configuration;
     }
 
     configPath = g_vm["config"].as<string>();
   }
   catch (const po::error &e) {
     g_log<<Logger::Error<<e.what()<<". See `ixfrdist --help` for valid options"<<endl;
-    return(EXIT_FAILURE);
+    return std::nullopt;
   }
   catch (const std::exception& exp) {
     g_log<<Logger::Error<<exp.what()<<". See `ixfrdist --help` for valid options"<<endl;
-    return(EXIT_FAILURE);
+    return std::nullopt;
   }
 
   bool had_error = false;
@@ -1240,16 +1254,16 @@ int main(int argc, char** argv) {
     YAML::Node config;
     if (!parseAndCheckConfig(configPath, config)) {
       // parseAndCheckConfig already logged whatever was wrong
-      return EXIT_FAILURE;
+      return std::nullopt;
     }
 
-  /*  From hereon out, we known that all the values in config are valid. */
+    /*  From hereon out, we known that all the values in config are valid. */
 
     for (auto const &domain : config["domains"]) {
       set<ComboAddress> s;
       s.insert(domain["master"].as<ComboAddress>());
       g_domainConfigs[domain["domain"].as<DNSName>()].masters = s;
-      if (domain["max-soa-refresh"]) {
+      if (domain["max-soa-refresh"].IsDefined()) {
         g_domainConfigs[domain["domain"].as<DNSName>()].maxSOARefresh = domain["max-soa-refresh"].as<uint32_t>();
       }
       g_stats.registerDomain(domain["domain"].as<DNSName>());
@@ -1276,20 +1290,13 @@ int main(int argc, char** argv) {
       g_log<<Logger::Error<<"Error printing ACL: "<<exp.what()<<endl;
     }
 
-    if (config["compress"]) {
+    if (config["compress"].IsDefined()) {
       g_compress = config["compress"].as<bool>();
       if (g_compress) {
         g_log<<Logger::Notice<<"Record compression is enabled."<<endl;
       }
     }
 
-    FDMultiplexer* fdm = FDMultiplexer::getMultiplexerSilent();
-    if (fdm == nullptr) {
-      g_log<<Logger::Error<<"Could not enable a multiplexer for the listen sockets!"<<endl;
-      return EXIT_FAILURE;
-    }
-
-    set<int> allSockets;
     for (const auto& addr : config["listen"].as<vector<ComboAddress>>()) {
       for (const auto& stype : {SOCK_DGRAM, SOCK_STREAM}) {
         try {
@@ -1300,8 +1307,8 @@ int main(int argc, char** argv) {
           if (stype == SOCK_STREAM) {
             SListen(s, 30); // TODO make this configurable
           }
-          fdm->addReadFD(s, stype == SOCK_DGRAM ? handleUDPRequest : handleTCPRequest);
-          allSockets.insert(s);
+          fdm.addReadFD(s, stype == SOCK_DGRAM ? handleUDPRequest : handleTCPRequest);
+          configuration.listeningSockets.insert(s);
         }
         catch (const runtime_error& exp) {
           g_log<<Logger::Error<<exp.what()<<endl;
@@ -1316,36 +1323,38 @@ int main(int argc, char** argv) {
       }
     }
 
-    int newgid = 0;
-
-    if (config["gid"]) {
-      string gid = config["gid"].as<string>();
-      if (!(newgid = atoi(gid.c_str()))) {
-        struct group *gr = getgrnam(gid.c_str());
+    if (config["gid"].IsDefined()) {
+      auto gid = config["gid"].as<string>();
+      try {
+        configuration.gid = pdns::checked_stoi<gid_t>(gid);
+      }
+      catch (const std::exception& e) {
+        g_log<<Logger::Error<<"Can not parse gid "<<gid<<endl;
+        had_error = true;
+      }
+      if (configuration.gid != 0) {
+        //NOLINTNEXTLINE(concurrency-mt-unsafe): only one thread at this point
+        const struct group *gr = getgrnam(gid.c_str());
         if (gr == nullptr) {
           g_log<<Logger::Error<<"Can not determine group-id for gid "<<gid<<endl;
           had_error = true;
         } else {
-          newgid = gr->gr_gid;
+          configuration.gid = gr->gr_gid;
         }
       }
-      g_log<<Logger::Notice<<"Dropping effective group-id to "<<newgid<<endl;
-      if (setgid(newgid) < 0) {
-        g_log<<Logger::Error<<"Could not set group id to "<<newgid<<": "<<stringerror()<<endl;
-        had_error = true;
-      }
     }
 
-    if (config["webserver-address"]) {
-      NetmaskGroup wsACL;
+    if (config["webserver-address"].IsDefined()) {
+      configuration.wsAddr = config["webserver-address"].as<ComboAddress>();
+
       try {
-        wsACL.addMask("127.0.0.0/8");
-        wsACL.addMask("::1/128");
+        configuration.wsACL.addMask("127.0.0.0/8");
+        configuration.wsACL.addMask("::1/128");
 
-        if (config["webserver-acl"]) {
-          wsACL.clear();
+        if (config["webserver-acl"].IsDefined()) {
+          configuration.wsACL.clear();
           for (const auto &acl : config["webserver-acl"].as<vector<Netmask>>()) {
-            wsACL.addMask(acl);
+            configuration.wsACL.addMask(acl);
           }
         }
       }
@@ -1354,95 +1363,153 @@ int main(int argc, char** argv) {
         had_error = true;
       }
 
-      string loglevel = "normal";
       if (config["webserver-loglevel"]) {
-        loglevel = config["webserver-loglevel"].as<string>();
+        configuration.wsLogLevel = config["webserver-loglevel"].as<string>();
       }
+    }
 
-      // Launch the webserver!
+    if (config["uid"].IsDefined()) {
+      string uid = config["uid"].as<string>();
       try {
-        std::thread(&IXFRDistWebServer::go, IXFRDistWebServer(config["webserver-address"].as<ComboAddress>(), wsACL, loglevel)).detach();
+        configuration.uid = pdns::checked_stoi<uid_t>(uid);
       }
-      catch (const std::exception& exp) {
-        g_log<<Logger::Error<<"Unable to start webserver: "<<exp.what()<<endl;
+      catch (const std::exception& e) {
+        g_log<<Logger::Error<<"Can not parse uid "<<uid<<endl;
         had_error = true;
       }
-      catch (const PDNSException &e) {
-        g_log<<Logger::Error<<"Unable to start webserver: "<<e.reason<<endl;
-        had_error = true;
-      }
-    }
-
-    int newuid = 0;
-
-    if (config["uid"]) {
-      string uid = config["uid"].as<string>();
-      if (!(newuid = atoi(uid.c_str()))) {
-        struct passwd *pw = getpwnam(uid.c_str());
+      if (configuration.uid != 0) {
+        //NOLINTNEXTLINE(concurrency-mt-unsafe): only one thread at this point
+        const struct passwd *pw = getpwnam(uid.c_str());
         if (pw == nullptr) {
           g_log<<Logger::Error<<"Can not determine user-id for uid "<<uid<<endl;
           had_error = true;
         } else {
-          newuid = pw->pw_uid;
+          configuration.uid = pw->pw_uid;
         }
       }
+    }
+
+    configuration.workDir = config["work-dir"].as<string>();
+    configuration.keep = config["keep"].as<uint16_t>();
+    configuration.axfrTimeout = config["axfr-timeout"].as<uint16_t>();
+    configuration.failedSOARetry = config["failed-soa-retry"].as<uint16_t>();
+    configuration.axfrMaxRecords = config["axfr-max-records"].as<uint32_t>();
+    configuration.tcpInThreads = config["tcp-in-threads"].as<uint16_t>();
+
+    if (had_error) {
+      return std::nullopt;
+    }
+    return configuration;
+  }
+  catch (const YAML::Exception& exp) {
+    had_error = true;
+    g_log<<Logger::Error<<"Got an exception while applying our configuration: "<<exp.msg<<endl;
+    return std::nullopt;
+  }
+}
+
+int main(int argc, char** argv) {
+  g_log.setLoglevel(Logger::Notice);
+  g_log.toConsole(Logger::Notice);
+  g_log.setPrefixed(true);
+  g_log.disableSyslog(true);
+  g_log.setTimestamps(false);
+
+  auto fdm = std::unique_ptr<FDMultiplexer>(FDMultiplexer::getMultiplexerSilent());
+  if (!fdm) {
+    g_log<<Logger::Error<<"Could not enable a multiplexer for the listen sockets!"<<endl;
+    return EXIT_FAILURE;
+  }
+
+  auto configuration = parseConfiguration(argc, argv, *fdm);
+
+  if (!configuration) {
+    // We have already sent the errors to stderr, just die
+    return EXIT_FAILURE;
+  }
+
+  if (configuration->done) {
+    return EXIT_SUCCESS;
+  }
+
+  bool had_error = false;
+  try {
+    if (configuration->gid != 0) {
+      g_log<<Logger::Notice<<"Dropping effective group-id to "<<configuration->gid<<endl;
+      if (setgid(configuration->gid) < 0) {
+        g_log<<Logger::Error<<"Could not set group id to "<<configuration->gid<<": "<<stringerror()<<endl;
+        had_error = true;
+      }
+    }
 
-      struct passwd *pw = getpwuid(newuid);
+    // It all starts here
+    signal(SIGTERM, handleSignal);
+    signal(SIGINT, handleSignal);
+    signal(SIGPIPE, SIG_IGN);
+
+    // Launch the webserver!
+    try {
+      std::thread(&IXFRDistWebServer::go, IXFRDistWebServer(configuration->wsAddr, configuration->wsACL, configuration->wsLogLevel)).detach();
+    }
+    catch (const std::exception& exp) {
+      g_log<<Logger::Error<<"Unable to start webserver: "<<exp.what()<<endl;
+      had_error = true;
+    }
+    catch (const PDNSException &e) {
+      g_log<<Logger::Error<<"Unable to start webserver: "<<e.reason<<endl;
+      had_error = true;
+    }
+
+    if (configuration->uid != 0) {
+      g_log<<Logger::Notice<<"Dropping effective user-id to "<<configuration->uid<<endl;
+      if (setuid(configuration->uid) < 0) {
+        g_log<<Logger::Error<<"Could not set user id to "<<configuration->uid<<": "<<stringerror()<<endl;
+        had_error = true;
+      }
+      const auto* pw = getpwuid(configuration->uid);
       if (pw == nullptr) {
         if (setgroups(0, nullptr) < 0) {
           g_log<<Logger::Error<<"Unable to drop supplementary gids: "<<stringerror()<<endl;
           had_error = true;
         }
       } else {
-        if (initgroups(pw->pw_name, newgid) < 0) {
+        if (initgroups(pw->pw_name, configuration->gid) < 0) {
           g_log<<Logger::Error<<"Unable to set supplementary groups: "<<stringerror()<<endl;
           had_error = true;
         }
       }
-
-      g_log<<Logger::Notice<<"Dropping effective user-id to "<<newuid<<endl;
-      if (setuid(newuid) < 0) {
-        g_log<<Logger::Error<<"Could not set user id to "<<newuid<<": "<<stringerror()<<endl;
-        had_error = true;
-      }
     }
 
     if (had_error) {
-      // We have already sent the errors to stderr, just die
       return EXIT_FAILURE;
     }
 
-    // It all starts here
-    signal(SIGTERM, handleSignal);
-    signal(SIGINT, handleSignal);
-    signal(SIGPIPE, SIG_IGN);
-
     // Init the things we need
     reportAllTypes();
 
     std::thread ut(updateThread,
-                   config["work-dir"].as<string>(),
-                   config["keep"].as<uint16_t>(),
-                   config["axfr-timeout"].as<uint16_t>(),
-                   config["failed-soa-retry"].as<uint16_t>(),
-                   config["axfr-max-records"].as<uint32_t>());
+                   configuration->workDir,
+                   configuration->keep,
+                   configuration->axfrTimeout,
+                   configuration->failedSOARetry,
+                   configuration->axfrMaxRecords);
 
     vector<std::thread> tcpHandlers;
-    tcpHandlers.reserve(config["tcp-in-threads"].as<uint16_t>());
+    tcpHandlers.reserve(configuration->tcpInThreads);
     for (size_t i = 0; i < tcpHandlers.capacity(); ++i) {
       tcpHandlers.push_back(std::thread(tcpWorker, i));
     }
 
     struct timeval now;
-    for(;;) {
+    for (;;) {
       gettimeofday(&now, 0);
       fdm->run(&now);
       if (g_exiting) {
         g_log<<Logger::Debug<<"Closing listening sockets"<<endl;
-        for (const int& fd : allSockets) {
+        for (const int& fd : configuration->listeningSockets) {
           try {
             closesocket(fd);
-          } catch(PDNSException &e) {
+          } catch (const PDNSException &e) {
             g_log<<Logger::Error<<e.reason<<endl;
           }
         }