nghttpx: Group up logging related options

src/shrpx.cc

@@ -929,13 +929,7 @@ void fill_default_config() {
   mod_config()->downstream_no_tls = false;
 
   mod_config()->num_worker = 1;
-  mod_config()->accesslog_file = nullptr;
-  mod_config()->accesslog_syslog = false;
-  mod_config()->accesslog_format = parse_log_format(DEFAULT_ACCESSLOG_FORMAT);
-  mod_config()->errorlog_file = strcopy("/dev/stderr");
-  mod_config()->errorlog_syslog = false;
   mod_config()->conf_path = strcopy("/etc/nghttpx/nghttpx.conf");
-  mod_config()->syslog_facility = LOG_DAEMON;
   // Default accept() backlog
   mod_config()->backlog = 512;
   mod_config()->http2_proxy = false;
@@ -1047,6 +1041,19 @@ void fill_default_config() {
   http2conf.max_concurrent_streams = 100;
   http2conf.no_cookie_crumbling = false;
   http2conf.no_server_push = false;
+
+  auto &loggingconf = mod_config()->logging;
+  {
+    auto &accessconf = loggingconf.access;
+    accessconf.syslog = false;
+    accessconf.format = parse_log_format(DEFAULT_ACCESSLOG_FORMAT);
+
+    auto &errorconf = loggingconf.error;
+    errorconf.file = strcopy("/dev/stderr");
+    errorconf.syslog = false;
+  }
+
+  loggingconf.syslog_facility = LOG_DAEMON;
 }
 
 } // namespace
@@ -1573,14 +1580,14 @@ Logging:
               Set path to write error  log.  To reopen file, send USR1
               signal  to nghttpx.   stderr will  be redirected  to the
               error log file unless --errorlog-syslog is used.
-              Default: )" << get_config()->errorlog_file.get() << R"(
+              Default: )" << get_config()->logging.error.file.get() << R"(
   --errorlog-syslog
               Send  error log  to  syslog.  If  this  option is  used,
               --errorlog-file option is ignored.
   --syslog-facility=<FACILITY>
               Set syslog facility to <FACILITY>.
-              Default: )" << str_syslog_facility(get_config()->syslog_facility)
+              Default: )"
-      << R"(
+      << str_syslog_facility(get_config()->logging.syslog_facility) << R"(
 
 HTTP:
   --add-x-forwarded-for
@@ -1759,9 +1766,11 @@ void process_options(
     assert(include_set.empty());
   }
 
-  if (get_config()->accesslog_syslog || get_config()->errorlog_syslog) {
+  auto &loggingconf = get_config()->logging;
+
+  if (loggingconf.access.syslog || loggingconf.error.syslog) {
     openlog("nghttpx", LOG_NDELAY | LOG_NOWAIT | LOG_PID,
-            get_config()->syslog_facility);
+            loggingconf.syslog_facility);
   }
 
   if (reopen_log_files() != 0) {

src/shrpx_client_handler.cc

@@ -836,7 +836,7 @@ void ClientHandler::write_accesslog(Downstream *downstream) {
   const auto &resp = downstream->response();
 
   upstream_accesslog(
-      get_config()->accesslog_format,
+      get_config()->logging.access.format,
       LogSpec{
           downstream, ipaddr_, http2::to_method_string(req.method),
 
@@ -869,7 +869,7 @@ void ClientHandler::write_accesslog(int major, int minor, unsigned int status,
   nghttp2::ssl::TLSSessionInfo tls_info;
 
   upstream_accesslog(
-      get_config()->accesslog_format,
+      get_config()->logging.access.format,
       LogSpec{
           nullptr, ipaddr_,
           StringRef::from_lit("-"), // method

src/shrpx_config.cc

@@ -1499,23 +1499,23 @@ int parse_config(const char *opt, const char *optarg,
   case SHRPX_OPTID_STREAM_WRITE_TIMEOUT:
     return parse_duration(&mod_config()->stream_write_timeout, opt, optarg);
   case SHRPX_OPTID_ACCESSLOG_FILE:
-    mod_config()->accesslog_file = strcopy(optarg);
+    mod_config()->logging.access.file = strcopy(optarg);
 
     return 0;
   case SHRPX_OPTID_ACCESSLOG_SYSLOG:
-    mod_config()->accesslog_syslog = util::strieq(optarg, "yes");
+    mod_config()->logging.access.syslog = util::strieq(optarg, "yes");
 
     return 0;
   case SHRPX_OPTID_ACCESSLOG_FORMAT:
-    mod_config()->accesslog_format = parse_log_format(optarg);
+    mod_config()->logging.access.format = parse_log_format(optarg);
 
     return 0;
   case SHRPX_OPTID_ERRORLOG_FILE:
-    mod_config()->errorlog_file = strcopy(optarg);
+    mod_config()->logging.error.file = strcopy(optarg);
 
     return 0;
   case SHRPX_OPTID_ERRORLOG_SYSLOG:
-    mod_config()->errorlog_syslog = util::strieq(optarg, "yes");
+    mod_config()->logging.error.syslog = util::strieq(optarg, "yes");
 
     return 0;
   case SHRPX_OPTID_FASTOPEN: {
@@ -1660,7 +1660,7 @@ int parse_config(const char *opt, const char *optarg,
       LOG(ERROR) << opt << ": Unknown syslog facility: " << optarg;
       return -1;
     }
-    mod_config()->syslog_facility = facility;
+    mod_config()->logging.syslog_facility = facility;
 
     return 0;
   }

src/shrpx_config.h

@@ -434,14 +434,29 @@ struct Http2Config {
   bool no_server_push;
 };
 
+struct LoggingConfig {
+  struct {
+    std::vector<LogFragment> format;
+    std::unique_ptr<char[]> file;
+    // Send accesslog to syslog, ignoring accesslog_file.
+    bool syslog;
+  } access;
+  struct {
+    std::unique_ptr<char[]> file;
+    // Send errorlog to syslog, ignoring errorlog_file.
+    bool syslog;
+  } error;
+  int syslog_facility;
+};
+
 struct Config {
-  std::vector<LogFragment> accesslog_format;
   std::vector<DownstreamAddrGroup> downstream_addr_groups;
   Router router;
   HttpProxy downstream_http_proxy;
   HttpConfig http;
   Http2Config http2;
   TLSConfig tls;
+  LoggingConfig logging;
   ev_tstamp http2_upstream_read_timeout;
   ev_tstamp upstream_read_timeout;
   ev_tstamp upstream_write_timeout;
@@ -456,8 +471,6 @@ struct Config {
   std::unique_ptr<char[]> host;
   std::unique_ptr<char[]> pid_file;
   std::unique_ptr<char[]> conf_path;
-  std::unique_ptr<char[]> accesslog_file;
-  std::unique_ptr<char[]> errorlog_file;
   std::unique_ptr<char[]> user;
   std::unique_ptr<char[]> mruby_file;
   char **original_argv;
@@ -483,7 +496,6 @@ struct Config {
   size_t downstream_addr_group_catch_all;
   // downstream protocol; this will be determined by given options.
   shrpx_proto downstream_proto;
-  int syslog_facility;
   int backlog;
   int argc;
   int fastopen;
@@ -500,10 +512,6 @@ struct Config {
   bool client_proxy;
   bool upstream_no_tls;
   bool downstream_no_tls;
-  // Send accesslog to syslog, ignoring accesslog_file.
-  bool accesslog_syslog;
-  // Send errorlog to syslog, ignoring errorlog_file.
-  bool errorlog_syslog;
   bool client;
   // true if --client or --client-proxy are enabled.
   bool client_mode;

src/shrpx_log.cc

@@ -109,12 +109,14 @@ Log::~Log() {
 
   auto lgconf = log_config();
 
+  auto &errorconf = get_config()->logging.error;
+
   if (!log_enabled(severity_) ||
-      (lgconf->errorlog_fd == -1 && !get_config()->errorlog_syslog)) {
+      (lgconf->errorlog_fd == -1 && !errorconf.syslog)) {
     return;
   }
 
-  if (get_config()->errorlog_syslog) {
+  if (errorconf.syslog) {
     if (severity_ == NOTICE) {
       syslog(severity_to_syslog_level(severity_), "[%s] %s",
              SEVERITY_STR[severity_], stream_.str().c_str());
@@ -219,8 +221,9 @@ std::pair<OutputIterator, size_t> copy_hex_low(const uint8_t *src,
 void upstream_accesslog(const std::vector<LogFragment> &lfv,
                         const LogSpec &lgsp) {
   auto lgconf = log_config();
+  auto &accessconf = get_config()->logging.access;
 
-  if (lgconf->accesslog_fd == -1 && !get_config()->accesslog_syslog) {
+  if (lgconf->accesslog_fd == -1 && !accessconf.syslog) {
     return;
   }
 
@@ -360,7 +363,7 @@ void upstream_accesslog(const std::vector<LogFragment> &lfv,
 
   *p = '\0';
 
-  if (get_config()->accesslog_syslog) {
+  if (accessconf.syslog) {
     syslog(LOG_INFO, "%s", buf);
 
     return;
@@ -379,29 +382,27 @@ int reopen_log_files() {
   int new_errorlog_fd = -1;
 
   auto lgconf = log_config();
+  auto &accessconf = get_config()->logging.access;
+  auto &errorconf = get_config()->logging.error;
 
-  if (!get_config()->accesslog_syslog && get_config()->accesslog_file) {
+  if (!accessconf.syslog && accessconf.file) {
-
+    new_accesslog_fd = util::open_log_file(accessconf.file.get());
-    new_accesslog_fd = util::open_log_file(get_config()->accesslog_file.get());
 
     if (new_accesslog_fd == -1) {
-      LOG(ERROR) << "Failed to open accesslog file "
+      LOG(ERROR) << "Failed to open accesslog file " << accessconf.file.get();
-                 << get_config()->accesslog_file.get();
       res = -1;
     }
   }
 
-  if (!get_config()->errorlog_syslog && get_config()->errorlog_file) {
+  if (!errorconf.syslog && errorconf.file) {
-
+    new_errorlog_fd = util::open_log_file(errorconf.file.get());
-    new_errorlog_fd = util::open_log_file(get_config()->errorlog_file.get());
 
     if (new_errorlog_fd == -1) {
       if (lgconf->errorlog_fd != -1) {
-        LOG(ERROR) << "Failed to open errorlog file "
+        LOG(ERROR) << "Failed to open errorlog file " << errorconf.file.get();
-                   << get_config()->errorlog_file.get();
       } else {
-        std::cerr << "Failed to open errorlog file "
+        std::cerr << "Failed to open errorlog file " << errorconf.file.get()
-                  << get_config()->errorlog_file.get() << std::endl;
+                  << std::endl;
       }
 
       res = -1;
@@ -444,8 +445,9 @@ void log_chld(pid_t pid, int rstatus, const char *msg) {
 
 void redirect_stderr_to_errorlog() {
   auto lgconf = log_config();
+  auto &errorconf = get_config()->logging.error;
 
-  if (get_config()->errorlog_syslog || lgconf->errorlog_fd == -1) {
+  if (errorconf.syslog || lgconf->errorlog_fd == -1) {
     return;
   }