diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index a70119e8a2..1c3bf9cbeb 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/scripts/maint/practracker/exceptions.txt
@@ -33,7 +33,7 @@
#
# Remember: It is better to fix the problem than to add a new exception!
-problem file-size /src/app/config/config.c 7212
+problem file-size /src/app/config/config.c 7400
problem include-count /src/app/config/config.c 80
problem function-size /src/app/config/config.c:options_act_reversible() 298
problem function-size /src/app/config/config.c:options_act() 381
diff --git a/src/app/config/config.c b/src/app/config/config.c
index 1ef23824d1..082dff3132 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -862,6 +862,8 @@ static void options_clear_cb(const config_mgr_t *mgr, void *opts);
static setopt_err_t options_validate_and_set(const or_options_t *old_options,
or_options_t *new_options,
char **msg_out);
+struct log_transaction_t;
+static void options_rollback_log_transaction(struct log_transaction_t *xn);
/** Magic value for or_options_t. */
#define OR_OPTIONS_MAGIC 9090909
@@ -1566,6 +1568,139 @@ options_create_directories(char **msg_out)
return 0;
}
+/** Structure to represent an incompleted configuration of a set of logs.
+ *
+ * This structure is generated by options_start_log_transaction(), and is
+ * either committed by options_commit_log_transaction() or rolled back by
+ * options_rollback_log_transaction(). */
+typedef struct log_transaction_t {
+ /** Previous lowest severity of any configured log. */
+ int old_min_log_level;
+ /** True if we have marked the previous logs to be closed */
+ bool logs_marked;
+ /** True if we initialized the new set of logs */
+ bool logs_initialized;
+ /** True if our safelogging configuration is different from what it was
+ * previously (or if we are starting for the first time). */
+ bool safelogging_changed;
+} log_transaction_t;
+
+/**
+ * Start configuring our logs based on the current value of get_options().
+ *
+ * The value old_options holds either the previous options object,
+ * or NULL if we're starting for the first time.
+ *
+ * On success, return a log_transaction_t that we can either roll back or
+ * commit.
+ *
+ * On failure return NULL and write a message into a newly allocated string in
+ * *msg_out.
+ **/
+static log_transaction_t *
+options_start_log_transaction(const or_options_t *old_options,
+ char **msg_out)
+{
+ const or_options_t *options = get_options();
+ const bool running_tor = options->command == CMD_RUN_TOR;
+
+ log_transaction_t *xn = tor_malloc_zero(sizeof(log_transaction_t));
+ xn->old_min_log_level = get_min_log_level();
+
+ if (! running_tor)
+ goto done;
+
+ mark_logs_temp(); /* Close current logs once new logs are open. */
+ xn->logs_marked = true;
+ /* Configure the tor_log(s) */
+ if (options_init_logs(old_options, options, 0)<0) {
+ *msg_out = tor_strdup("Failed to init Log options. See logs for details.");
+ options_rollback_log_transaction(xn);
+ xn = NULL;
+ goto done;
+ }
+
+ xn->safelogging_changed = !old_options ||
+ old_options->SafeLogging_ != options->SafeLogging_;
+
+ xn->logs_initialized = true;
+
+ done:
+ return xn;
+}
+
+/**
+ * Finish configuring the logs that started to get configured with xn.
+ * Frees xn.
+ **/
+static void
+options_commit_log_transaction(log_transaction_t *xn)
+{
+ const or_options_t *options = get_options();
+ tor_assert(xn);
+
+ if (xn->logs_marked) {
+ log_severity_list_t *severity =
+ tor_malloc_zero(sizeof(log_severity_list_t));
+ close_temp_logs();
+ add_callback_log(severity, control_event_logmsg);
+ logs_set_pending_callback_callback(control_event_logmsg_pending);
+ control_adjust_event_log_severity();
+ tor_free(severity);
+ tor_log_update_sigsafe_err_fds();
+ }
+
+ if (xn->logs_initialized) {
+ flush_log_messages_from_startup();
+ }
+
+ {
+ const char *badness = NULL;
+ int bad_safelog = 0, bad_severity = 0, new_badness = 0;
+ if (options->SafeLogging_ != SAFELOG_SCRUB_ALL) {
+ bad_safelog = 1;
+ if (xn->safelogging_changed)
+ new_badness = 1;
+ }
+ if (get_min_log_level() >= LOG_INFO) {
+ bad_severity = 1;
+ if (get_min_log_level() != xn->old_min_log_level)
+ new_badness = 1;
+ }
+ if (bad_safelog && bad_severity)
+ badness = "you disabled SafeLogging, and "
+ "you're logging more than \"notice\"";
+ else if (bad_safelog)
+ badness = "you disabled SafeLogging";
+ else
+ badness = "you're logging more than \"notice\"";
+ if (new_badness)
+ log_warn(LD_GENERAL, "Your log may contain sensitive information - %s. "
+ "Don't log unless it serves an important reason. "
+ "Overwrite the log afterwards.", badness);
+ }
+
+ tor_free(xn);
+}
+
+/**
+ * Revert the log configuration changes that that started to get configured
+ * with xn. Frees xn.
+ **/
+static void
+options_rollback_log_transaction(log_transaction_t *xn)
+{
+ if (!xn)
+ return;
+
+ if (xn->logs_marked) {
+ rollback_log_changes();
+ control_adjust_event_log_severity();
+ }
+
+ tor_free(xn);
+}
+
/** Fetch the active option list, and take actions based on it. All of the
* things we do should survive being done repeatedly. If present,
* old_options contains the previous value of the options.
@@ -1587,10 +1722,9 @@ 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;
+ log_transaction_t *log_transaction = NULL;
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;
@@ -1663,62 +1797,16 @@ options_act_reversible,(const or_options_t *old_options, char **msg))
goto done;
}
-
/* Bail out at this point if we're not going to be a client or server:
* we don't run Tor itself. */
- if (!running_tor)
- goto commit;
-
- mark_logs_temp(); /* Close current logs once new logs are open. */
- logs_marked = 1;
- /* Configure the tor_log(s) */
- if (options_init_logs(old_options, options, 0)<0) {
- *msg = tor_strdup("Failed to init Log options. See logs for details.");
+ log_transaction = options_start_log_transaction(old_options, msg);
+ if (log_transaction == NULL)
goto rollback;
- }
- logs_initialized = 1;
- commit:
+ // Commit!
r = 0;
- if (logs_marked) {
- log_severity_list_t *severity =
- tor_malloc_zero(sizeof(log_severity_list_t));
- close_temp_logs();
- add_callback_log(severity, control_event_logmsg);
- logs_set_pending_callback_callback(control_event_logmsg_pending);
- control_adjust_event_log_severity();
- tor_free(severity);
- tor_log_update_sigsafe_err_fds();
- }
- if (logs_initialized) {
- flush_log_messages_from_startup();
- }
- {
- const char *badness = NULL;
- int bad_safelog = 0, bad_severity = 0, new_badness = 0;
- if (options->SafeLogging_ != SAFELOG_SCRUB_ALL) {
- bad_safelog = 1;
- if (!old_options || old_options->SafeLogging_ != options->SafeLogging_)
- new_badness = 1;
- }
- if (get_min_log_level() >= LOG_INFO) {
- bad_severity = 1;
- if (get_min_log_level() != old_min_log_level)
- new_badness = 1;
- }
- if (bad_safelog && bad_severity)
- badness = "you disabled SafeLogging, and "
- "you're logging more than \"notice\"";
- else if (bad_safelog)
- badness = "you disabled SafeLogging";
- else
- badness = "you're logging more than \"notice\"";
- if (new_badness)
- log_warn(LD_GENERAL, "Your log may contain sensitive information - %s. "
- "Don't log unless it serves an important reason. "
- "Overwrite the log afterwards.", badness);
- }
+ options_commit_log_transaction(log_transaction);
if (set_conn_limit) {
/*
@@ -1754,10 +1842,7 @@ options_act_reversible,(const or_options_t *old_options, char **msg))
r = -1;
tor_assert(*msg);
- if (logs_marked) {
- rollback_log_changes();
- control_adjust_event_log_severity();
- }
+ options_rollback_log_transaction(log_transaction);
if (set_conn_limit && old_options)
set_max_file_descriptors((unsigned)old_options->ConnLimit,
@@ -4857,7 +4942,7 @@ options_init_log_granularity(const or_options_t *options,
* Initialize the logs based on the configuration file.
*/
STATIC int
-options_init_logs(const or_options_t *old_options, or_options_t *options,
+options_init_logs(const or_options_t *old_options, const or_options_t *options,
int validate_only)
{
config_line_t *opt;
diff --git a/src/app/config/config.h b/src/app/config/config.h
index eeba9e64d0..0af96a0c26 100644
--- a/src/app/config/config.h
+++ b/src/app/config/config.h
@@ -301,7 +301,7 @@ STATIC int open_and_add_file_log(const log_severity_list_t *severity,
const char *fname,
int truncate_log);
STATIC int options_init_logs(const or_options_t *old_options,
- or_options_t *options, int validate_only);
+ const or_options_t *options, int validate_only);
#ifdef TOR_UNIT_TESTS
int options_validate(const or_options_t *old_options,