[tor-commits] [tor/master] Extract a function for one-time-only pre-reversible options.

[nickm at torproject.org]

commit 006ce47ffa0dbc60dc0e3fc5d58e24943e291ef9
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Nov 7 12:47:20 2019 -0500

    Extract a function for one-time-only pre-reversible options.
    
    These changes _only_ happen at startup, and happen before _any_
    reversible option change is set.
---
 src/app/config/config.c | 109 ++++++++++++++++++++++++++++--------------------
 1 file changed, 64 insertions(+), 45 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index ff9bf833f..57835f95d 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -904,8 +904,8 @@ static smartlist_t *configured_ports = NULL;
 /** True iff we're currently validating options, and any calls to
  * get_options() are likely to be bugs. */
 static int in_option_validation = 0;
-/* True iff we've initialized libevent */
-static int libevent_initialized = 0;
+/** True iff we have run options_act_once_on_startup() */
+static bool have_set_startup_options = false;
 
 /* A global configuration manager to handle all configuration objects. */
 static config_mgr_t *options_mgr = NULL;
@@ -1085,7 +1085,7 @@ config_free_all(void)
 
   cleanup_protocol_warning_severity_level();
 
-  libevent_initialized = 0;
+  have_set_startup_options = false;
 
   config_mgr_free(options_mgr);
 }
@@ -1422,27 +1422,24 @@ create_keys_directory(const or_options_t *options)
 /* Helps determine flags to pass to switch_id. */
 static int have_low_ports = -1;
 
-/** Fetch the active option list, and take actions based on it. All of the
- * things we do should survive being done repeatedly.  If present,
- * <b>old_options</b> contains the previous value of the options.
- *
- * Return 0 if all goes well, return -1 if things went badly.
- */
-MOCK_IMPL(STATIC int,
-options_act_reversible,(const or_options_t *old_options, char **msg))
+/** Take case of initial startup tasks that must occur before any of the
+ * transactional option-related changes are allowed. */
+static int
+options_act_once_on_startup(char **msg_out)
 {
-  smartlist_t *new_listeners = smartlist_new();
-  or_options_t *options = get_options_mutable();
+  if (have_set_startup_options)
+    return 0;
+
+  const or_options_t *options = get_options();
   int running_tor = options->command == CMD_RUN_TOR;
-  int set_conn_limit = 0;
-  int r = -1;
-  int logs_marked = 0, logs_initialized = 0;
-  int old_min_log_level = get_min_log_level();
+
+  if (!running_tor)
+    return 0;
 
   /* Daemonize _first_, since we only want to open most of this stuff in
    * the subprocess.  Libevent bases can't be reliably inherited across
    * processes. */
-  if (running_tor && options->RunAsDaemon) {
+  if (options->RunAsDaemon) {
     if (! start_daemon_has_been_called())
       subsystems_prefork();
     /* No need to roll back, since you can't change the value. */
@@ -1455,6 +1452,55 @@ options_act_reversible,(const or_options_t *old_options, char **msg))
   sd_notifyf(0, "MAINPID=%ld\n", (long int)getpid());
 #endif
 
+  /* Set up libevent.  (We need to do this before we can register the
+   * listeners as listeners.) */
+  init_libevent(options);
+
+  /* This has to come up after libevent is initialized. */
+  control_initialize_event_queue();
+
+  /*
+   * Initialize the scheduler - this has to come after
+   * options_init_from_torrc() sets up libevent - why yes, that seems
+   * completely sensible to hide the libevent setup in the option parsing
+   * code!  It also needs to happen before init_keys(), so it needs to
+   * happen here too.  How yucky. */
+  scheduler_init();
+
+  /* Attempt to lock all current and future memory with mlockall() only once.
+   * This must happen before setuid. */
+  if (options->DisableAllSwap) {
+    if (tor_mlockall() == -1) {
+      *msg_out = tor_strdup("DisableAllSwap failure. Do you have proper "
+                        "permissions?");
+      return -1;
+    }
+  }
+
+  have_set_startup_options = true;
+  return 0;
+}
+
+/** Fetch the active option list, and take actions based on it. All of the
+ * things we do should survive being done repeatedly.  If present,
+ * <b>old_options</b> contains the previous value of the options.
+ *
+ * Return 0 if all goes well, return -1 if things went badly.
+ */
+MOCK_IMPL(STATIC int,
+options_act_reversible,(const or_options_t *old_options, char **msg))
+{
+  smartlist_t *new_listeners = smartlist_new();
+  or_options_t *options = get_options_mutable();
+  int running_tor = options->command == CMD_RUN_TOR;
+  int set_conn_limit = 0;
+  int r = -1;
+  int logs_marked = 0, logs_initialized = 0;
+  int old_min_log_level = get_min_log_level();
+
+  if (options_act_once_on_startup(msg) < 0)
+    goto rollback;
+
   if (running_tor) {
     int n_ports=0;
     /* We need to set the connection limit before we can open the listeners. */
@@ -1471,24 +1517,6 @@ options_act_reversible,(const or_options_t *old_options, char **msg))
       options->ConnLimit_ = old_options->ConnLimit_;
     }
 
-    /* Set up libevent.  (We need to do this before we can register the
-     * listeners as listeners.) */
-    if (running_tor && !libevent_initialized) {
-      init_libevent(options);
-      libevent_initialized = 1;
-
-      /* This has to come up after libevent is initialized. */
-      control_initialize_event_queue();
-
-      /*
-       * Initialize the scheduler - this has to come after
-       * options_init_from_torrc() sets up libevent - why yes, that seems
-       * completely sensible to hide the libevent setup in the option parsing
-       * code!  It also needs to happen before init_keys(), so it needs to
-       * happen here too.  How yucky. */
-      scheduler_init();
-    }
-
     /* Adjust the port configuration so we can launch listeners. */
     /* 31851: some ports are relay-only */
     if (parse_ports(options, 0, msg, &n_ports, NULL)) {
@@ -1533,15 +1561,6 @@ options_act_reversible,(const or_options_t *old_options, char **msg))
   }
 #endif /* defined(HAVE_NET_IF_H) && defined(HAVE_NET_PFVAR_H) */
 
-  /* Attempt to lock all current and future memory with mlockall() only once */
-  if (options->DisableAllSwap) {
-    if (tor_mlockall() == -1) {
-      *msg = tor_strdup("DisableAllSwap failure. Do you have proper "
-                        "permissions?");
-      goto done;
-    }
-  }
-
   /* Setuid/setgid as appropriate */
   if (options->User) {
     tor_assert(have_low_ports != -1);