Loading changes/smartlist_foreach 0 → 100644 +8 −0 Original line number Diff line number Diff line o Code simplification and refactoring: - Do not use SMARTLIST_FOREACH for any loop whose body exceeds 10 lines. Doing so in the past has led to hard-to-debug code. The new style is to use the SMARTLIST_FOREACH_{BEGIN,END} pair. Issue 6400. - Do not nest SMARTLIST_FOREACH blocks within one another. Any nested block ought to be using SMARTLIST_FOREACH_{BEGIN,END}. Issue 6400. src/common/container.h +15 −9 Original line number Diff line number Diff line Loading @@ -142,9 +142,9 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join, /** Iterate over the items in a smartlist <b>sl</b>, in order. For each item, * assign it to a new local variable of type <b>type</b> named <b>var</b>, and * execute the statement <b>cmd</b>. Inside the loop, the loop index can * be accessed as <b>var</b>_sl_idx and the length of the list can be accessed * as <b>var</b>_sl_len. * execute the statements inside the loop body. Inside the loop, the loop * index can be accessed as <b>var</b>_sl_idx and the length of the list can * be accessed as <b>var</b>_sl_len. * * NOTE: Do not change the length of the list while the loop is in progress, * unless you adjust the _sl_len variable correspondingly. See second example Loading @@ -153,23 +153,21 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join, * Example use: * <pre> * smartlist_t *list = smartlist_split("A:B:C", ":", 0, 0); * SMARTLIST_FOREACH(list, char *, cp, * { * SMARTLIST_FOREACH_BEGIN(list, char *, cp) { * printf("%d: %s\n", cp_sl_idx, cp); * tor_free(cp); * }); * } SMARTLIST_FOREACH_END(cp); * smartlist_free(list); * </pre> * * Example use (advanced): * <pre> * SMARTLIST_FOREACH(list, char *, cp, * { * SMARTLIST_FOREACH_BEGIN(list, char *, cp) { * if (!strcmp(cp, "junk")) { * tor_free(cp); * SMARTLIST_DEL_CURRENT(list, cp); * } * }); * } SMARTLIST_FOREACH_END(cp); * </pre> */ /* Note: these macros use token pasting, and reach into smartlist internals. Loading Loading @@ -218,6 +216,14 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join, var = NULL; \ } STMT_END /** * An alias for SMARTLIST_FOREACH_BEGIN and SMARTLIST_FOREACH_END, using * <b>cmd</b> as the loop body. This wrapper is here for convenience with * very short loops. * * By convention, we do not use this for loops which nest, or for loops over * 10 lines or so. Use SMARTLIST_FOREACH_{BEGIN,END} for those. */ #define SMARTLIST_FOREACH(sl, type, var, cmd) \ SMARTLIST_FOREACH_BEGIN(sl,type,var) { \ cmd; \ Loading src/common/log.c +2 −3 Original line number Diff line number Diff line Loading @@ -1005,8 +1005,7 @@ parse_log_severity_config(const char **cfg_ptr, smartlist_split_string(domains_list, domains_str, ",", SPLIT_SKIP_SPACE, -1); tor_free(domains_str); SMARTLIST_FOREACH(domains_list, const char *, domain, { SMARTLIST_FOREACH_BEGIN(domains_list, const char *, domain) { if (!strcmp(domain, "*")) { domains = ~0u; } else { Loading @@ -1027,7 +1026,7 @@ parse_log_severity_config(const char **cfg_ptr, domains |= d; } } }); } SMARTLIST_FOREACH_END(domain); SMARTLIST_FOREACH(domains_list, char *, d, tor_free(d)); smartlist_free(domains_list); if (err) Loading src/or/circuitbuild.c +6 −8 Original line number Diff line number Diff line Loading @@ -4323,7 +4323,7 @@ entry_guard_register_connect_status(const char *digest, int succeeded, * came back? We should give our earlier entries another try too, * and close this connection so we don't use it before we've given * the others a shot. */ SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, { SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, e) { if (e == entry) break; if (e->made_contact) { Loading @@ -4334,7 +4334,7 @@ entry_guard_register_connect_status(const char *digest, int succeeded, e->can_retry = 1; } } }); } SMARTLIST_FOREACH_END(e); if (refuse_conn) { log_info(LD_CIRC, "Connected to new entry guard '%s' (%s). Marking earlier " Loading Loading @@ -4804,8 +4804,7 @@ entry_guards_update_state(or_state_t *state) *next = NULL; if (!entry_guards) entry_guards = smartlist_new(); SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, { SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, e) { char dbuf[HEX_DIGEST_LEN+1]; if (!e->made_contact) continue; /* don't write this one to disk */ Loading Loading @@ -4852,7 +4851,7 @@ entry_guards_update_state(or_state_t *state) next = &(line->next); } }); } SMARTLIST_FOREACH_END(e); if (!get_options()->AvoidDiskWrites) or_state_mark_dirty(get_or_state(), 0); entry_guards_dirty = 0; Loading Loading @@ -5494,8 +5493,7 @@ int any_pending_bridge_descriptor_fetches(void) { smartlist_t *conns = get_connection_array(); SMARTLIST_FOREACH(conns, connection_t *, conn, { SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) { if (conn->type == CONN_TYPE_DIR && conn->purpose == DIR_PURPOSE_FETCH_SERVERDESC && TO_DIR_CONN(conn)->router_purpose == ROUTER_PURPOSE_BRIDGE && Loading @@ -5505,7 +5503,7 @@ any_pending_bridge_descriptor_fetches(void) log_debug(LD_DIR, "found one: %s", conn->address); return 1; } }); } SMARTLIST_FOREACH_END(conn); return 0; } Loading src/or/config.c +4 −4 Original line number Diff line number Diff line Loading @@ -3288,7 +3288,7 @@ compute_publishserverdescriptor(or_options_t *options) *auth = NO_DIRINFO; if (!list) /* empty list, answer is none */ return 0; SMARTLIST_FOREACH(list, const char *, string, { SMARTLIST_FOREACH_BEGIN(list, const char *, string) { if (!strcasecmp(string, "v1")) *auth |= V1_DIRINFO; else if (!strcmp(string, "1")) Loading @@ -3310,7 +3310,7 @@ compute_publishserverdescriptor(or_options_t *options) /* no authority */; else return -1; }); } SMARTLIST_FOREACH_END(string); return 0; } Loading Loading @@ -3646,7 +3646,7 @@ options_validate(or_options_t *old_options, or_options_t *options, options->_AllowInvalid = 0; if (options->AllowInvalidNodes) { SMARTLIST_FOREACH(options->AllowInvalidNodes, const char *, cp, { SMARTLIST_FOREACH_BEGIN(options->AllowInvalidNodes, const char *, cp) { if (!strcasecmp(cp, "entry")) options->_AllowInvalid |= ALLOW_INVALID_ENTRY; else if (!strcasecmp(cp, "exit")) Loading @@ -3662,7 +3662,7 @@ options_validate(or_options_t *old_options, or_options_t *options, "Unrecognized value '%s' in AllowInvalidNodes", cp); return -1; } }); } SMARTLIST_FOREACH_END(cp); } if (!options->SafeLogging || Loading Loading
changes/smartlist_foreach 0 → 100644 +8 −0 Original line number Diff line number Diff line o Code simplification and refactoring: - Do not use SMARTLIST_FOREACH for any loop whose body exceeds 10 lines. Doing so in the past has led to hard-to-debug code. The new style is to use the SMARTLIST_FOREACH_{BEGIN,END} pair. Issue 6400. - Do not nest SMARTLIST_FOREACH blocks within one another. Any nested block ought to be using SMARTLIST_FOREACH_{BEGIN,END}. Issue 6400.
src/common/container.h +15 −9 Original line number Diff line number Diff line Loading @@ -142,9 +142,9 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join, /** Iterate over the items in a smartlist <b>sl</b>, in order. For each item, * assign it to a new local variable of type <b>type</b> named <b>var</b>, and * execute the statement <b>cmd</b>. Inside the loop, the loop index can * be accessed as <b>var</b>_sl_idx and the length of the list can be accessed * as <b>var</b>_sl_len. * execute the statements inside the loop body. Inside the loop, the loop * index can be accessed as <b>var</b>_sl_idx and the length of the list can * be accessed as <b>var</b>_sl_len. * * NOTE: Do not change the length of the list while the loop is in progress, * unless you adjust the _sl_len variable correspondingly. See second example Loading @@ -153,23 +153,21 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join, * Example use: * <pre> * smartlist_t *list = smartlist_split("A:B:C", ":", 0, 0); * SMARTLIST_FOREACH(list, char *, cp, * { * SMARTLIST_FOREACH_BEGIN(list, char *, cp) { * printf("%d: %s\n", cp_sl_idx, cp); * tor_free(cp); * }); * } SMARTLIST_FOREACH_END(cp); * smartlist_free(list); * </pre> * * Example use (advanced): * <pre> * SMARTLIST_FOREACH(list, char *, cp, * { * SMARTLIST_FOREACH_BEGIN(list, char *, cp) { * if (!strcmp(cp, "junk")) { * tor_free(cp); * SMARTLIST_DEL_CURRENT(list, cp); * } * }); * } SMARTLIST_FOREACH_END(cp); * </pre> */ /* Note: these macros use token pasting, and reach into smartlist internals. Loading Loading @@ -218,6 +216,14 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join, var = NULL; \ } STMT_END /** * An alias for SMARTLIST_FOREACH_BEGIN and SMARTLIST_FOREACH_END, using * <b>cmd</b> as the loop body. This wrapper is here for convenience with * very short loops. * * By convention, we do not use this for loops which nest, or for loops over * 10 lines or so. Use SMARTLIST_FOREACH_{BEGIN,END} for those. */ #define SMARTLIST_FOREACH(sl, type, var, cmd) \ SMARTLIST_FOREACH_BEGIN(sl,type,var) { \ cmd; \ Loading
src/common/log.c +2 −3 Original line number Diff line number Diff line Loading @@ -1005,8 +1005,7 @@ parse_log_severity_config(const char **cfg_ptr, smartlist_split_string(domains_list, domains_str, ",", SPLIT_SKIP_SPACE, -1); tor_free(domains_str); SMARTLIST_FOREACH(domains_list, const char *, domain, { SMARTLIST_FOREACH_BEGIN(domains_list, const char *, domain) { if (!strcmp(domain, "*")) { domains = ~0u; } else { Loading @@ -1027,7 +1026,7 @@ parse_log_severity_config(const char **cfg_ptr, domains |= d; } } }); } SMARTLIST_FOREACH_END(domain); SMARTLIST_FOREACH(domains_list, char *, d, tor_free(d)); smartlist_free(domains_list); if (err) Loading
src/or/circuitbuild.c +6 −8 Original line number Diff line number Diff line Loading @@ -4323,7 +4323,7 @@ entry_guard_register_connect_status(const char *digest, int succeeded, * came back? We should give our earlier entries another try too, * and close this connection so we don't use it before we've given * the others a shot. */ SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, { SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, e) { if (e == entry) break; if (e->made_contact) { Loading @@ -4334,7 +4334,7 @@ entry_guard_register_connect_status(const char *digest, int succeeded, e->can_retry = 1; } } }); } SMARTLIST_FOREACH_END(e); if (refuse_conn) { log_info(LD_CIRC, "Connected to new entry guard '%s' (%s). Marking earlier " Loading Loading @@ -4804,8 +4804,7 @@ entry_guards_update_state(or_state_t *state) *next = NULL; if (!entry_guards) entry_guards = smartlist_new(); SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, { SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, e) { char dbuf[HEX_DIGEST_LEN+1]; if (!e->made_contact) continue; /* don't write this one to disk */ Loading Loading @@ -4852,7 +4851,7 @@ entry_guards_update_state(or_state_t *state) next = &(line->next); } }); } SMARTLIST_FOREACH_END(e); if (!get_options()->AvoidDiskWrites) or_state_mark_dirty(get_or_state(), 0); entry_guards_dirty = 0; Loading Loading @@ -5494,8 +5493,7 @@ int any_pending_bridge_descriptor_fetches(void) { smartlist_t *conns = get_connection_array(); SMARTLIST_FOREACH(conns, connection_t *, conn, { SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) { if (conn->type == CONN_TYPE_DIR && conn->purpose == DIR_PURPOSE_FETCH_SERVERDESC && TO_DIR_CONN(conn)->router_purpose == ROUTER_PURPOSE_BRIDGE && Loading @@ -5505,7 +5503,7 @@ any_pending_bridge_descriptor_fetches(void) log_debug(LD_DIR, "found one: %s", conn->address); return 1; } }); } SMARTLIST_FOREACH_END(conn); return 0; } Loading
src/or/config.c +4 −4 Original line number Diff line number Diff line Loading @@ -3288,7 +3288,7 @@ compute_publishserverdescriptor(or_options_t *options) *auth = NO_DIRINFO; if (!list) /* empty list, answer is none */ return 0; SMARTLIST_FOREACH(list, const char *, string, { SMARTLIST_FOREACH_BEGIN(list, const char *, string) { if (!strcasecmp(string, "v1")) *auth |= V1_DIRINFO; else if (!strcmp(string, "1")) Loading @@ -3310,7 +3310,7 @@ compute_publishserverdescriptor(or_options_t *options) /* no authority */; else return -1; }); } SMARTLIST_FOREACH_END(string); return 0; } Loading Loading @@ -3646,7 +3646,7 @@ options_validate(or_options_t *old_options, or_options_t *options, options->_AllowInvalid = 0; if (options->AllowInvalidNodes) { SMARTLIST_FOREACH(options->AllowInvalidNodes, const char *, cp, { SMARTLIST_FOREACH_BEGIN(options->AllowInvalidNodes, const char *, cp) { if (!strcasecmp(cp, "entry")) options->_AllowInvalid |= ALLOW_INVALID_ENTRY; else if (!strcasecmp(cp, "exit")) Loading @@ -3662,7 +3662,7 @@ options_validate(or_options_t *old_options, or_options_t *options, "Unrecognized value '%s' in AllowInvalidNodes", cp); return -1; } }); } SMARTLIST_FOREACH_END(cp); } if (!options->SafeLogging || Loading