From 35746a9ee75608e7b2393e4519adca602bf2615f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 19 Oct 2017 13:42:28 -0400 Subject: [PATCH] Comment-only change: annotate exit() calls. Sometimes when we call exit(), it's because the process is completely hopeless: openssl has a broken AES-CTR implementation, or the clock is in the 1960s, or something like that. But sometimes, we should return cleanly from tor_main() instead, so that embedders can keep embedding us and start another Tor process. I've gone through all the exit() and _exit() calls to annotate them with "exit ok" or "XXXX bad exit" -- the next step will be to fix the bad exit()s. First step towards 23848. --- src/common/aes.c | 2 +- src/common/compat_libevent.c | 2 +- src/common/compat_time.c | 4 ++-- src/common/compat_winthreads.c | 2 +- src/common/sandbox.c | 2 +- src/common/util.c | 24 ++++++++++++------------ src/or/config.c | 24 ++++++++++++------------ src/or/control.c | 2 +- src/or/hibernate.c | 4 ++-- src/or/main.c | 10 +++++----- src/or/networkstatus.c | 2 +- src/or/ntmain.c | 2 +- src/or/routerlist.c | 4 ++-- src/or/scheduler.c | 2 +- 14 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/common/aes.c b/src/common/aes.c index 20b51a6758..df4368fdba 100644 --- a/src/common/aes.c +++ b/src/common/aes.c @@ -254,7 +254,7 @@ evaluate_ctr_for_aes(void) /* LCOV_EXCL_START */ log_err(LD_CRYPTO, "This OpenSSL has a buggy version of counter mode; " "quitting tor."); - exit(1); + exit(1); // exit ok: openssl is broken. /* LCOV_EXCL_STOP */ } return 0; diff --git a/src/common/compat_libevent.c b/src/common/compat_libevent.c index 740cc2a11d..1c3a1b9f37 100644 --- a/src/common/compat_libevent.c +++ b/src/common/compat_libevent.c @@ -126,7 +126,7 @@ tor_libevent_initialize(tor_libevent_cfg *torcfg) if (!the_event_base) { /* LCOV_EXCL_START */ log_err(LD_GENERAL, "Unable to initialize Libevent: cannot continue."); - exit(1); + exit(1); // exit ok: libevent is broken. /* LCOV_EXCL_STOP */ } diff --git a/src/common/compat_time.c b/src/common/compat_time.c index 1ce6f5ce4e..7fd4281ade 100644 --- a/src/common/compat_time.c +++ b/src/common/compat_time.c @@ -90,7 +90,7 @@ tor_gettimeofday(struct timeval *timeval) if (ft.ft_64 < EPOCH_BIAS) { /* LCOV_EXCL_START */ log_err(LD_GENERAL,"System time is before 1970; failing."); - exit(1); + exit(1); // exit ok: system clock is broken. /* LCOV_EXCL_STOP */ } ft.ft_64 -= EPOCH_BIAS; @@ -102,7 +102,7 @@ tor_gettimeofday(struct timeval *timeval) log_err(LD_GENERAL,"gettimeofday failed."); /* If gettimeofday dies, we have either given a bad timezone (we didn't), or segfaulted.*/ - exit(1); + exit(1); // exit ok: gettimeofday failed. /* LCOV_EXCL_STOP */ } #elif defined(HAVE_FTIME) diff --git a/src/common/compat_winthreads.c b/src/common/compat_winthreads.c index 122e0f0166..5f7ec94c23 100644 --- a/src/common/compat_winthreads.c +++ b/src/common/compat_winthreads.c @@ -52,7 +52,7 @@ spawn_exit(void) //we should never get here. my compiler thinks that _endthread returns, this //is an attempt to fool it. tor_assert(0); - _exit(0); + _exit(0); // exit ok: unreachable. // LCOV_EXCL_STOP } diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 7a4e3ece38..931837e767 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -1756,7 +1756,7 @@ sigsys_debugging(int nr, siginfo_t *info, void *void_context) #endif #if defined(DEBUGGING_CLOSE) - _exit(1); + _exit(1); // exit ok: programming error has led to sandbox failure. #endif // DEBUGGING_CLOSE } diff --git a/src/common/util.c b/src/common/util.c index 5ff7e104d6..7dc5e8144d 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -156,7 +156,7 @@ tor_malloc_(size_t size DMALLOC_PARAMS) /* If these functions die within a worker process, they won't call * spawn_exit, but that's ok, since the parent will run out of memory soon * anyway. */ - exit(1); + exit(1); // exit ok: alloc failed. /* LCOV_EXCL_STOP */ } return result; @@ -244,7 +244,7 @@ tor_realloc_(void *ptr, size_t size DMALLOC_PARAMS) if (PREDICT_UNLIKELY(result == NULL)) { /* LCOV_EXCL_START */ log_err(LD_MM,"Out of memory on realloc(). Dying."); - exit(1); + exit(1); // exit ok: alloc failed. /* LCOV_EXCL_STOP */ } return result; @@ -282,7 +282,7 @@ tor_strdup_(const char *s DMALLOC_PARAMS) if (PREDICT_UNLIKELY(duplicate == NULL)) { /* LCOV_EXCL_START */ log_err(LD_MM,"Out of memory on strdup(). Dying."); - exit(1); + exit(1); // exit ok: alloc failed. /* LCOV_EXCL_STOP */ } return duplicate; @@ -3590,14 +3590,14 @@ start_daemon(void) if (pipe(daemon_filedes)) { /* LCOV_EXCL_START */ log_err(LD_GENERAL,"pipe failed; exiting. Error was %s", strerror(errno)); - exit(1); + exit(1); // exit ok: during daemonize, pipe failed. /* LCOV_EXCL_STOP */ } pid = fork(); if (pid < 0) { /* LCOV_EXCL_START */ log_err(LD_GENERAL,"fork failed. Exiting."); - exit(1); + exit(1); // exit ok: during daemonize, fork failed /* LCOV_EXCL_STOP */ } if (pid) { /* Parent */ @@ -3612,9 +3612,9 @@ start_daemon(void) } fflush(stdout); if (ok == 1) - exit(0); + exit(0); // exit ok: during daemonize, daemonizing. else - exit(1); /* child reported error */ + exit(1); /* child reported error. exit ok: daemonize failed. */ } else { /* Child */ close(daemon_filedes[0]); /* we only write */ @@ -3626,7 +3626,7 @@ start_daemon(void) * _Advanced Programming in the Unix Environment_. */ if (fork() != 0) { - exit(0); + exit(0); // exit ok: during daemonize, fork failed (2) } set_main_thread(); /* We are now the main thread. */ @@ -3655,14 +3655,14 @@ finish_daemon(const char *desired_cwd) /* Don't hold the wrong FS mounted */ if (chdir(desired_cwd) < 0) { log_err(LD_GENERAL,"chdir to \"%s\" failed. Exiting.",desired_cwd); - exit(1); + exit(1); // exit ok: during daemonize, chdir failed. } nullfd = tor_open_cloexec("/dev/null", O_RDWR, 0); if (nullfd < 0) { /* LCOV_EXCL_START */ log_err(LD_GENERAL,"/dev/null can't be opened. Exiting."); - exit(1); + exit(1); // exit ok: during daemonize, couldn't open /dev/null /* LCOV_EXCL_STOP */ } /* close fds linking to invoking terminal, but @@ -3674,7 +3674,7 @@ finish_daemon(const char *desired_cwd) dup2(nullfd,2) < 0) { /* LCOV_EXCL_START */ log_err(LD_GENERAL,"dup2 failed. Exiting."); - exit(1); + exit(1); // exit ok: during daemonize, dup2 failed. /* LCOV_EXCL_STOP */ } if (nullfd > 2) @@ -4474,7 +4474,7 @@ tor_spawn_background(const char *const filename, const char **argv, err += (nbytes < 0); } - _exit(err?254:255); + _exit(err?254:255); // exit ok: in child. } /* Never reached, but avoids compiler warning */ diff --git a/src/or/config.c b/src/or/config.c index 832a7c9674..55ab76c813 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -837,7 +837,7 @@ set_options(or_options_t *new_val, char **msg) if (options_act(old_options) < 0) { /* acting on the options failed. die. */ log_err(LD_BUG, "Acting on config options left us in a broken state. Dying."); - exit(1); + exit(1); // XXXX bad exit } /* Issues a CONF_CHANGED event to notify controller of the change. If Tor is * just starting up then the old_options will be undefined. */ @@ -5008,22 +5008,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); + exit(0); // XXXX bad exit, though probably harmless } if (config_line_find(cmdline_only_options, "--list-torrc-options")) { /* For validating whether we've documented everything. */ list_torrc_options(); - exit(0); + exit(0); // XXXX bad exit, though probably harmless } if (config_line_find(cmdline_only_options, "--list-deprecated-options")) { /* For validating whether what we have deprecated really exists. */ list_deprecated_options(); - exit(0); + exit(0); // XXXX bad exit, though probably harmless } if (config_line_find(cmdline_only_options, "--version")) { printf("Tor version %s.\n",get_version()); - exit(0); + exit(0); // XXXX bad exit, though probably harmless } if (config_line_find(cmdline_only_options, "--library-versions")) { @@ -5051,7 +5051,7 @@ options_init_from_torrc(int argc, char **argv) tor_compress_header_version_str(ZSTD_METHOD)); } //TODO: Hex versions? - exit(0); + exit(0); // XXXX bad exit, though probably harmless } command = CMD_RUN_TOR; @@ -5112,7 +5112,7 @@ 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); + exit(1); // XXXX bad exit } } @@ -5121,7 +5121,7 @@ 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); + exit(1); // XXXX bad exit } } @@ -5131,17 +5131,17 @@ 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); + exit(1); // XXXX bad exit } else if (command != CMD_KEYGEN) { log_err(LD_CONFIG, "--passphrase-fd specified without --keygen!"); - exit(1); + exit(1); // XXXX bad exit } 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); + exit(1); // XXXX bad exit } get_options_mutable()->keygen_passphrase_fd = (int)fd; get_options_mutable()->use_keygen_passphrase_fd = 1; @@ -5156,7 +5156,7 @@ 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); + exit(1); // XXXX bad exit } else { get_options_mutable()->master_key_fname = tor_strdup(key_line->value); } diff --git a/src/or/control.c b/src/or/control.c index 8173cb1e56..ab164700e6 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -6572,7 +6572,7 @@ monitor_owning_controller_process(const char *process_spec) msg); owning_controller_process_spec = NULL; tor_cleanup(); - exit(1); + exit(1); // XXXX bad exit: or questionable, at least. } } diff --git a/src/or/hibernate.c b/src/or/hibernate.c index 74ab766468..be2bc7ce95 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -819,7 +819,7 @@ hibernate_begin(hibernate_state_t new_state, time_t now) hibernate_state == HIBERNATE_STATE_EXITING ? "a second time" : "while hibernating"); tor_cleanup(); - exit(0); + exit(0); // XXXX bad exit } if (new_state == HIBERNATE_STATE_LOWBANDWIDTH && @@ -981,7 +981,7 @@ consider_hibernation(time_t now) if (shutdown_time <= now) { log_notice(LD_GENERAL, "Clean shutdown finished. Exiting."); tor_cleanup(); - exit(0); + exit(0); // XXXX bad exit } return; /* if exiting soon, don't worry about bandwidth limits */ } diff --git a/src/or/main.c b/src/or/main.c index 65b0b8f4df..be61628341 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1555,7 +1555,7 @@ check_ed_keys_callback(time_t now, const or_options_t *options) generate_ed_link_cert(options, now, new_signing_key > 0)) { log_err(LD_OR, "Unable to update Ed25519 keys! Exiting."); tor_cleanup(); - exit(1); + exit(1); // XXXX bad exit } } return 30; @@ -2709,13 +2709,13 @@ process_signal(int sig) case SIGTERM: log_notice(LD_GENERAL,"Catching signal TERM, exiting cleanly."); tor_cleanup(); - exit(0); + exit(0); // XXXX bad exit break; case SIGINT: if (!server_mode(get_options())) { /* do it now */ log_notice(LD_GENERAL,"Interrupt: exiting cleanly."); tor_cleanup(); - exit(0); + exit(0); // XXXX bad exit } #ifdef HAVE_SYSTEMD sd_notify(0, "STOPPING=1"); @@ -2745,7 +2745,7 @@ process_signal(int sig) if (do_hup() < 0) { log_warn(LD_CONFIG,"Restart failed (config error?). Exiting."); tor_cleanup(); - exit(1); + exit(1); // XXXX bad exit } #ifdef HAVE_SYSTEMD sd_notify(0, "READY=1"); @@ -3198,7 +3198,7 @@ try_locking(const or_options_t *options, int err_if_locked) r = try_locking(options, 0); if (r<0) { log_err(LD_GENERAL, "No, it's still there. Exiting."); - exit(1); + exit(1); // XXXX bad exit } return r; } diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 93bb8643d8..3a4f06fb7b 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1671,7 +1671,7 @@ handle_missing_protocol_warning_impl(const networkstatus_t *c, } tor_free(protocol_warning); if (should_exit) - exit(1); + exit(1); // XXXX bad exit: should return from main. } /** Called when we have received a networkstatus c. If there are diff --git a/src/or/ntmain.c b/src/or/ntmain.c index 508e5844eb..9ce03e1f5a 100644 --- a/src/or/ntmain.c +++ b/src/or/ntmain.c @@ -195,7 +195,7 @@ nt_service_loadlibrary(void) return; err: printf("Unable to load library support for NT services: exiting.\n"); - exit(1); + exit(1); // exit ok: ntmain can't read libraries } /** If we're compiled to run as an NT service, and the service wants to diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 1fa27f722c..b59f318fc8 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -3082,7 +3082,7 @@ signed_descriptor_get_body_impl(const signed_descriptor_t *desc, log_err(LD_DIR, "We couldn't read a descriptor that is supposedly " "mmaped in our cache. Is another process running in our data " "directory? Exiting."); - exit(1); + exit(1); // XXXX bad exit: should recover. } } if (!r) /* no mmap, or not in cache. */ @@ -3096,7 +3096,7 @@ signed_descriptor_get_body_impl(const signed_descriptor_t *desc, log_err(LD_DIR, "descriptor at %p begins with unexpected string %s. " "Is another process running in our data directory? Exiting.", desc, escaped(cp)); - exit(1); + exit(1); // XXXX bad exit: should recover. } } diff --git a/src/or/scheduler.c b/src/or/scheduler.c index 22a9b574ac..3ac3f406af 100644 --- a/src/or/scheduler.c +++ b/src/or/scheduler.c @@ -281,7 +281,7 @@ select_scheduler(void) * wishes of using what it has been configured and don't do a sneaky * fallback. Because this can be changed at runtime, we have to stop tor * right now. */ - exit(1); + exit(1); // XXXX bad exit } /* Set the chosen scheduler. */