Browse Source

Return instead of exiting in options_init_from_torrc()

Nick Mathewson 6 years ago
parent
commit
1df43aff41
4 changed files with 45 additions and 20 deletions
  1. 19 12
      src/or/config.c
  2. 1 2
      src/or/control.c
  3. 24 5
      src/or/main.c
  4. 1 1
      src/or/ntmain.c

+ 19 - 12
src/or/config.c

@@ -4981,7 +4981,8 @@ load_torrc_from_disk(config_line_t *cmd_arg, int defaults_file)
 /** Read a configuration file into <b>options</b>, finding the configuration
  * file location based on the command line.  After loading the file
  * call options_init_from_string() to load the config.
- * Return 0 if success, -1 if failure. */
+ * Return 0 if success, -1 if failure, and 1 if we succeeded but should exit
+ * anyway. */
 int
 options_init_from_torrc(int argc, char **argv)
 {
@@ -5008,22 +5009,22 @@ options_init_from_torrc(int argc, char **argv)
   if (config_line_find(cmdline_only_options, "-h") ||
       config_line_find(cmdline_only_options, "--help")) {
     print_usage();
-    exit(0); // XXXX bad exit, though probably harmless
+    return 1;
   }
   if (config_line_find(cmdline_only_options, "--list-torrc-options")) {
     /* For validating whether we've documented everything. */
     list_torrc_options();
-    exit(0); // XXXX bad exit, though probably harmless
+    return 1;
   }
   if (config_line_find(cmdline_only_options, "--list-deprecated-options")) {
     /* For validating whether what we have deprecated really exists. */
     list_deprecated_options();
-    exit(0); // XXXX bad exit, though probably harmless
+    return 1;
   }
 
   if (config_line_find(cmdline_only_options, "--version")) {
     printf("Tor version %s.\n",get_version());
-    exit(0); // XXXX bad exit, though probably harmless
+    return 1;
   }
 
   if (config_line_find(cmdline_only_options, "--library-versions")) {
@@ -5051,7 +5052,7 @@ options_init_from_torrc(int argc, char **argv)
                         tor_compress_header_version_str(ZSTD_METHOD));
     }
     //TODO: Hex versions?
-    exit(0); // XXXX bad exit, though probably harmless
+    return 1;
   }
 
   command = CMD_RUN_TOR;
@@ -5112,7 +5113,8 @@ options_init_from_torrc(int argc, char **argv)
       get_options_mutable()->keygen_force_passphrase = FORCE_PASSPHRASE_OFF;
     } else {
       log_err(LD_CONFIG, "--no-passphrase specified without --keygen!");
-      exit(1); // XXXX bad exit
+      retval = -1;
+      goto err;
     }
   }
 
@@ -5121,7 +5123,8 @@ options_init_from_torrc(int argc, char **argv)
       get_options_mutable()->change_key_passphrase = 1;
     } else {
       log_err(LD_CONFIG, "--newpass specified without --keygen!");
-      exit(1); // XXXX bad exit
+      retval = -1;
+      goto err;
     }
   }
 
@@ -5131,17 +5134,20 @@ options_init_from_torrc(int argc, char **argv)
     if (fd_line) {
       if (get_options()->keygen_force_passphrase == FORCE_PASSPHRASE_OFF) {
         log_err(LD_CONFIG, "--no-passphrase specified with --passphrase-fd!");
-        exit(1); // XXXX bad exit
+        retval = -1;
+        goto err;
       } else if (command != CMD_KEYGEN) {
         log_err(LD_CONFIG, "--passphrase-fd specified without --keygen!");
-        exit(1); // XXXX bad exit
+        retval = -1;
+        goto err;
       } else {
         const char *v = fd_line->value;
         int ok = 1;
         long fd = tor_parse_long(v, 10, 0, INT_MAX, &ok, NULL);
         if (fd < 0 || ok == 0) {
           log_err(LD_CONFIG, "Invalid --passphrase-fd value %s", escaped(v));
-          exit(1); // XXXX bad exit
+          retval = -1;
+          goto err;
         }
         get_options_mutable()->keygen_passphrase_fd = (int)fd;
         get_options_mutable()->use_keygen_passphrase_fd = 1;
@@ -5156,7 +5162,8 @@ options_init_from_torrc(int argc, char **argv)
     if (key_line) {
       if (command != CMD_KEYGEN) {
         log_err(LD_CONFIG, "--master-key without --keygen!");
-        exit(1); // XXXX bad exit
+        retval = -1;
+        goto err;
       } else {
         get_options_mutable()->master_key_fname = tor_strdup(key_line->value);
       }

+ 1 - 2
src/or/control.c

@@ -6571,8 +6571,7 @@ monitor_owning_controller_process(const char *process_spec)
             "owning controller: %s.  Exiting.",
             msg);
     owning_controller_process_spec = NULL;
-    tor_cleanup();
-    exit(1); // XXXX bad exit: or questionable, at least.
+    tor_shutdown_event_loop_and_exit(1);
   }
 }
 

+ 24 - 5
src/or/main.c

@@ -2420,10 +2420,18 @@ do_hup(void)
   /* first, reload config variables, in case they've changed */
   if (options->ReloadTorrcOnSIGHUP) {
     /* no need to provide argc/v, they've been cached in init_from_config */
-    if (options_init_from_torrc(0, NULL) < 0) {
+    int init_rv = options_init_from_torrc(0, NULL);
+    if (init_rv < 0) {
       log_err(LD_CONFIG,"Reading config failed--see warnings above. "
               "For usage, try -h.");
       return -1;
+    } else if (BUG(init_rv > 0)) {
+      // LCOV_EXCL_START
+      /* This should be impossible: the only "return 1" cases in
+       * options_init_from_torrc are ones caused by command-line arguments;
+       * but they can't change while Tor is running. */
+      return -1;
+      // LCOV_EXCL_STOP
     }
     options = get_options(); /* they have changed now */
     /* Logs are only truncated the first time they are opened, but were
@@ -3085,7 +3093,8 @@ activate_signal(int signal_num)
   }
 }
 
-/** Main entry point for the Tor command-line client.
+/** Main entry point for the Tor command-line client.  Return 0 on "success",
+ * negative on "failure", and positive on "success and exit".
  */
 int
 tor_init(int argc, char *argv[])
@@ -3192,9 +3201,14 @@ tor_init(int argc, char *argv[])
   }
   atexit(exit_function);
 
-  if (options_init_from_torrc(argc,argv) < 0) {
+  int init_rv = options_init_from_torrc(argc,argv);
+  if (init_rv < 0) {
     log_err(LD_CONFIG,"Reading config failed--see warnings above.");
     return -1;
+  } else if (init_rv > 0) {
+    // We succeeded, and should exit anyway -- probably the user just said
+    // "--version" or something like that.
+    return 1;
   }
 
   /* The options are now initialised */
@@ -3820,8 +3834,13 @@ tor_main(int argc, char *argv[])
      if (done) return result;
   }
 #endif /* defined(NT_SERVICE) */
-  if (tor_init(argc, argv)<0)
-    return -1;
+  {
+    int init_rv = tor_init(argc, argv);
+    if (init_rv < 0)
+      return -1;
+    else if (init_rv > 0)
+      return 0;
+  }
 
   if (get_options()->Sandbox && get_options()->command == CMD_RUN_TOR) {
     sandbox_cfg_t* cfg = sandbox_init_filter();

+ 1 - 1
src/or/ntmain.c

@@ -318,7 +318,7 @@ nt_service_main(void)
     printf("Service error %d : %s\n", (int) result, errmsg);
     tor_free(errmsg);
     if (result == ERROR_FAILED_SERVICE_CONTROLLER_CONNECT) {
-      if (tor_init(backup_argc, backup_argv) < 0)
+      if (tor_init(backup_argc, backup_argv))
         return;
       switch (get_options()->command) {
       case CMD_RUN_TOR: