Merge branch 'smartlist_shorten' into maint-0.2.3

21 files changed, 166 insertions, 193 deletions

diff --git a/changes/smartlist_foreach b/changes/smartlist_foreach
new file mode 100644
index 0000000000..2fd3a1a85c
--- /dev/null
+++ b/changes/smartlist_foreach
@@ -0,0 +1,8 @@
+  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.
diff --git a/src/common/container.h b/src/common/container.h
index c11c2eb82f..dab3b83f37 100644
--- a/src/common/container.h
+++ b/src/common/container.h
@@ -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
@@ -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.
@@ -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;                                                        \
diff --git a/src/common/log.c b/src/common/log.c
index 5966e44459..5e2e6b5b50 100644
--- a/src/common/log.c
+++ b/src/common/log.c
@@ -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 {
@@ -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)
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 714bbca34e..b82cce9881 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -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) {
@@ -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 "
@@ -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 */
@@ -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;
@@ -5687,8 +5686,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 &&
@@ -5698,7 +5696,7 @@ any_pending_bridge_descriptor_fetches(void)
       log_debug(LD_DIR, "found one: %s", conn->address);
       return 1;
     }
-  });
+  } SMARTLIST_FOREACH_END(conn);
   return 0;
 }
 
diff --git a/src/or/config.c b/src/or/config.c
index bacb06ee99..8703667945 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -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"))
@@ -3310,7 +3310,7 @@ compute_publishserverdescriptor(or_options_t *options)
       /* no authority */;
     else
       return -1;
-    });
+  } SMARTLIST_FOREACH_END(string);
   return 0;
 }
 
@@ -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"))
@@ -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 ||
diff --git a/src/or/connection.c b/src/or/connection.c
index 95101ef65d..364e4912da 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -706,8 +706,7 @@ connection_expire_held_open(void)
 
   now = time(NULL);
 
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
+  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
     /* If we've been holding the connection open, but we haven't written
      * for 15 seconds...
      */
@@ -729,7 +728,7 @@ connection_expire_held_open(void)
         conn->hold_open_until_flushed = 0;
       }
     }
-  });
+  } SMARTLIST_FOREACH_END(conn);
 }
 
 #if defined(HAVE_SYS_UN_H) || defined(RUNNING_DOXYGEN)
@@ -2477,8 +2476,7 @@ connection_bucket_refill(int milliseconds_elapsed, time_t now)
                                   "global_relayed_write_bucket");
 
   /* refill the per-connection buckets */
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
+  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
     if (connection_speaks_cells(conn)) {
       or_connection_t *or_conn = TO_OR_CONN(conn);
       int orbandwidthrate = or_conn->bandwidthrate;
@@ -2525,7 +2523,7 @@ connection_bucket_refill(int milliseconds_elapsed, time_t now)
       conn->write_blocked_on_bw = 0;
       connection_start_writing(conn);
     }
-  });
+  } SMARTLIST_FOREACH_END(conn);
 }
 
 /** Is the <b>bucket</b> for connection <b>conn</b> low enough that we
@@ -3974,8 +3972,7 @@ connection_dump_buffer_mem_stats(int severity)
   memset(alloc_by_type, 0, sizeof(alloc_by_type));
   memset(n_conns_by_type, 0, sizeof(n_conns_by_type));
 
-  SMARTLIST_FOREACH(conns, connection_t *, c,
-  {
+  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, c) {
     int tp = c->type;
     ++n_conns_by_type[tp];
     if (c->inbuf) {
@@ -3986,7 +3983,7 @@ connection_dump_buffer_mem_stats(int severity)
       used_by_type[tp] += buf_datalen(c->outbuf);
       alloc_by_type[tp] += buf_allocation(c->outbuf);
     }
-  });
+  } SMARTLIST_FOREACH_END(c);
   for (i=0; i <= _CONN_TYPE_MAX; ++i) {
     total_used += used_by_type[i];
     total_alloc += alloc_by_type[i];
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 3c8b4bcc92..1592033c54 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -647,8 +647,7 @@ connection_ap_attach_pending(void)
 {
   entry_connection_t *entry_conn;
   smartlist_t *conns = get_connection_array();
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
+  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
     if (conn->marked_for_close ||
         conn->type != CONN_TYPE_AP ||
         conn->state != AP_CONN_STATE_CIRCUIT_WAIT)
@@ -659,7 +658,7 @@ connection_ap_attach_pending(void)
         connection_mark_unattached_ap(entry_conn,
                                       END_STREAM_REASON_CANT_ATTACH);
     }
-  });
+  } SMARTLIST_FOREACH_END(conn);
 }
 
 /** Tell any AP streams that are waiting for a one-hop tunnel to
diff --git a/src/or/control.c b/src/or/control.c
index 6675c01b63..ce571f99f3 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -820,8 +820,7 @@ handle_control_getconf(control_connection_t *conn, uint32_t body_len,
   (void) body_len; /* body is NUL-terminated; so we can ignore len. */
   smartlist_split_string(questions, body, " ",
                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH(questions, const char *, q,
-  {
+  SMARTLIST_FOREACH_BEGIN(questions, const char *, q) {
     if (!option_is_recognized(q)) {
       smartlist_add(unrecognized, (char*) q);
     } else {
@@ -843,7 +842,7 @@ handle_control_getconf(control_connection_t *conn, uint32_t body_len,
         answer = next;
       }
     }
-  });
+  } SMARTLIST_FOREACH_END(q);
 
   if ((len = smartlist_len(unrecognized))) {
     for (i=0; i < len-1; ++i)
@@ -1644,8 +1643,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
     routerlist_t *routerlist = router_get_routerlist();
     smartlist_t *sl = smartlist_new();
     if (routerlist && routerlist->routers) {
-      SMARTLIST_FOREACH(routerlist->routers, const routerinfo_t *, ri,
-      {
+      SMARTLIST_FOREACH_BEGIN(routerlist->routers, const routerinfo_t *, ri) {
         const char *body = signed_descriptor_get_body(&ri->cache_info);
         signed_descriptor_t *ei = extrainfo_get_by_descriptor_digest(
                                      ri->cache_info.extra_info_digest);
@@ -1656,7 +1654,7 @@ getinfo_helper_dir(control_connection_t *control_conn,
           smartlist_add(sl,
                   tor_strndup(body, ri->cache_info.signed_descriptor_len));
         }
-      });
+      } SMARTLIST_FOREACH_END(ri);
     }
     *answer = smartlist_join_strings(sl, "", 0, NULL);
     SMARTLIST_FOREACH(sl, char *, c, tor_free(c));
@@ -2450,8 +2448,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
   smartlist_free(args);
 
   nodes = smartlist_new();
-  SMARTLIST_FOREACH(router_nicknames, const char *, n,
-  {
+  SMARTLIST_FOREACH_BEGIN(router_nicknames, const char *, n) {
     const node_t *node = node_get_by_nickname(n, 1);
     if (!node) {
       connection_printf_to_buf(conn, "552 No such router \"%s\"\r\n", n);
@@ -2462,7 +2459,7 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len,
       goto done;
     }
     smartlist_add(nodes, (void*)node);
-  });
+  } SMARTLIST_FOREACH_END(n);
   if (!smartlist_len(nodes)) {
     connection_write_str_to_buf("512 No router names provided\r\n", conn);
     goto done;
@@ -2686,8 +2683,7 @@ handle_control_postdescriptor(control_connection_t *conn, uint32_t len,
 
   smartlist_split_string(args, body, " ",
                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH(args, char *, option,
-  {
+  SMARTLIST_FOREACH_BEGIN(args, char *, option) {
     if (!strcasecmpstart(option, "purpose=")) {
       option += strlen("purpose=");
       purpose = router_purpose_from_string(option);
@@ -2712,7 +2708,7 @@ handle_control_postdescriptor(control_connection_t *conn, uint32_t len,
         "512 Unexpected argument \"%s\" to postdescriptor\r\n", option);
       goto done;
     }
-  });
+  } SMARTLIST_FOREACH_END(option);
 
   read_escaped_data(cp, len-(cp-body), &desc);
 
@@ -3110,7 +3106,7 @@ handle_control_usefeature(control_connection_t *conn,
   args = smartlist_new();
   smartlist_split_string(args, body, " ",
                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-  SMARTLIST_FOREACH(args, const char *, arg, {
+  SMARTLIST_FOREACH_BEGIN(args, const char *, arg) {
       if (!strcasecmp(arg, "VERBOSE_NAMES"))
         ;
       else if (!strcasecmp(arg, "EXTENDED_EVENTS"))
@@ -3121,7 +3117,7 @@ handle_control_usefeature(control_connection_t *conn,
         bad = 1;
         break;
       }
-    });
+  } SMARTLIST_FOREACH_END(arg);
 
   if (!bad) {
     send_control_done(conn);
diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c
index 9d48dd7cf7..0255227e7b 100644
--- a/src/or/cpuworker.c
+++ b/src/or/cpuworker.c
@@ -417,8 +417,7 @@ cull_wedged_cpuworkers(void)
 {
   time_t now = time(NULL);
   smartlist_t *conns = get_connection_array();
-  SMARTLIST_FOREACH(conns, connection_t *, conn,
-  {
+  SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
     if (!conn->marked_for_close &&
         conn->type == CONN_TYPE_CPUWORKER &&
         conn->state == CPUWORKER_STATE_BUSY_ONION &&
@@ -429,7 +428,7 @@ cull_wedged_cpuworkers(void)
       num_cpuworkers--;
       connection_mark_for_close(conn);
     }
-  });
+  } SMARTLIST_FOREACH_END(conn);
 }
 
 /** Try to tell a cpuworker to perform the public key operations necessary to
diff --git a/src/or/directory.c b/src/or/directory.c
index 1fb4835d0f..f58aab256b 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -535,9 +535,8 @@ directory_get_from_all_authorities(uint8_t dir_purpose,
   tor_assert(dir_purpose == DIR_PURPOSE_FETCH_STATUS_VOTE ||
              dir_purpose == DIR_PURPOSE_FETCH_DETACHED_SIGNATURES);
 
-  SMARTLIST_FOREACH(router_get_trusted_dir_servers(),
-                    trusted_dir_server_t *, ds,
-    {
+  SMARTLIST_FOREACH_BEGIN(router_get_trusted_dir_servers(),
+                          trusted_dir_server_t *, ds) {
       routerstatus_t *rs;
       if (router_digest_is_me(ds->digest))
         continue;
@@ -546,7 +545,7 @@ directory_get_from_all_authorities(uint8_t dir_purpose,
       rs = &ds->fake_status;
       directory_initiate_command_routerstatus(rs, dir_purpose, router_purpose,
                                               0, resource, NULL, 0, 0);
-    });
+  } SMARTLIST_FOREACH_END(ds);
 }
 
 /** Same as directory_initiate_command_routerstatus(), but accepts
@@ -1092,9 +1091,8 @@ directory_get_consensus_url(int supports_conditional_consensus,
     char *authority_id_list;
     smartlist_t *authority_digests = smartlist_new();
 
-    SMARTLIST_FOREACH(router_get_trusted_dir_servers(),
-                      trusted_dir_server_t *, ds,
-      {
+    SMARTLIST_FOREACH_BEGIN(router_get_trusted_dir_servers(),
+                            trusted_dir_server_t *, ds) {
         char *hex;
         if (!(ds->type & V3_DIRINFO))
           continue;
@@ -1103,7 +1101,7 @@ directory_get_consensus_url(int supports_conditional_consensus,
         base16_encode(hex, 2*CONDITIONAL_CONSENSUS_FPR_LEN+1,
                       ds->v3_identity_digest, CONDITIONAL_CONSENSUS_FPR_LEN);
         smartlist_add(authority_digests, hex);
-      });
+    } SMARTLIST_FOREACH_END(ds);
     smartlist_sort(authority_digests, _compare_strs);
     authority_id_list = smartlist_join_strings(authority_digests,
                                                "+", 0, NULL);
@@ -3603,8 +3601,7 @@ dir_networkstatus_download_failed(smartlist_t *failed, int status_code)
 {
   if (status_code == 503)
     return;
-  SMARTLIST_FOREACH(failed, const char *, fp,
-  {
+  SMARTLIST_FOREACH_BEGIN(failed, const char *, fp) {
     char digest[DIGEST_LEN];
     trusted_dir_server_t *dir;
     if (base16_decode(digest, DIGEST_LEN, fp, strlen(fp))<0) {
@@ -3616,7 +3613,7 @@ dir_networkstatus_download_failed(smartlist_t *failed, int status_code)
 
     if (dir)
       download_status_failed(&dir->v2_ns_dl_status, status_code);
-  });
+  } SMARTLIST_FOREACH_END(fp);
 }
 
 /** Schedule for when servers should download things in general. */
@@ -3770,8 +3767,7 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code,
     }
     return; /* FFFF should implement for other-than-router-purpose someday */
   }
-  SMARTLIST_FOREACH(failed, const char *, cp,
-  {
+  SMARTLIST_FOREACH_BEGIN(failed, const char *, cp) {
     download_status_t *dls = NULL;
     if (base16_decode(digest, DIGEST_LEN, cp, strlen(cp)) < 0) {
       log_warn(LD_BUG, "Malformed fingerprint in list: %s", escaped(cp));
@@ -3788,7 +3784,7 @@ dir_routerdesc_download_failed(smartlist_t *failed, int status_code,
     if (!dls || dls->n_download_failures >= MAX_ROUTERDESC_DOWNLOAD_FAILURES)
       continue;
     download_status_increment_failure(dls, status_code, cp, server, now);
-  });
+  } SMARTLIST_FOREACH_END(cp);
 
   /* No need to relaunch descriptor downloads here: we already do it
    * every 10 or 60 seconds (FOO_DESCRIPTOR_RETRY_INTERVAL) in main.c. */
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index bfebbcdfbd..e21f5113f2 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -2293,8 +2293,7 @@ get_possible_sybil_list(const smartlist_t *routers)
 
   last_addr = 0;
   addr_count = 0;
-  SMARTLIST_FOREACH(routers_by_ip, routerinfo_t *, ri,
-    {
+  SMARTLIST_FOREACH_BEGIN(routers_by_ip, routerinfo_t *, ri) {
       if (last_addr != ri->addr) {
         last_addr = ri->addr;
         addr_count = 1;
@@ -2303,7 +2302,7 @@ get_possible_sybil_list(const smartlist_t *routers)
             addr_count > max_with_same_addr_on_authority)
           digestmap_set(omit_as_sybil, ri->cache_info.identity_digest, ri);
       }
-    });
+  } SMARTLIST_FOREACH_END(ri);
 
   smartlist_free(routers_by_ip);
   return omit_as_sybil;
@@ -2964,7 +2963,7 @@ generate_v2_networkstatus_opinion(void)
 
   omit_as_sybil = get_possible_sybil_list(routers);
 
-  SMARTLIST_FOREACH(routers, routerinfo_t *, ri, {
+  SMARTLIST_FOREACH_BEGIN(routers, routerinfo_t *, ri) {
     if (ri->cache_info.published_on >= cutoff) {
       routerstatus_t rs;
       char *version = version_from_platform(ri->platform);
@@ -2988,7 +2987,7 @@ generate_v2_networkstatus_opinion(void)
       tor_free(version);
       outp += strlen(outp);
     }
-  });
+  } SMARTLIST_FOREACH_END(ri);
 
   if (tor_snprintf(outp, endp-outp, "directory-signature %s\n",
                    options->Nickname)<0) {
@@ -3106,8 +3105,7 @@ dirserv_get_networkstatus_v2(smartlist_t *result,
     cached_v2_networkstatus = digestmap_new();
 
   dirserv_get_networkstatus_v2_fingerprints(fingerprints, key);
-  SMARTLIST_FOREACH(fingerprints, const char *, fp,
-    {
+  SMARTLIST_FOREACH_BEGIN(fingerprints, const char *, fp) {
       if (router_digest_is_me(fp) && should_generate_v2_networkstatus())
         generate_v2_networkstatus_opinion();
       cached = digestmap_get(cached_v2_networkstatus, fp);
@@ -3119,7 +3117,7 @@ dirserv_get_networkstatus_v2(smartlist_t *result,
         log_info(LD_DIRSERV, "Don't know about any network status with "
                  "fingerprint '%s'", hexbuf);
       }
-    });
+  } SMARTLIST_FOREACH_END(fp);
   SMARTLIST_FOREACH(fingerprints, char *, cp, tor_free(cp));
   smartlist_free(fingerprints);
 }
@@ -3238,8 +3236,7 @@ dirserv_get_routerdescs(smartlist_t *descs_out, const char *key,
     key += strlen("/tor/server/fp/");
     dir_split_resource_into_fingerprints(key, digests, NULL,
                                          DSR_HEX|DSR_SORT_UNIQ);
-    SMARTLIST_FOREACH(digests, const char *, d,
-       {
+    SMARTLIST_FOREACH_BEGIN(digests, const char *, d) {
          if (router_digest_is_me(d)) {
            /* make sure desc_routerinfo exists */
            const routerinfo_t *ri = router_get_my_routerinfo();
@@ -3254,7 +3251,7 @@ dirserv_get_routerdescs(smartlist_t *descs_out, const char *key,
            if (ri && ri->cache_info.published_on > cutoff)
              smartlist_add(descs_out, (void*) &(ri->cache_info));
          }
-       });
+    } SMARTLIST_FOREACH_END(d);
     SMARTLIST_FOREACH(digests, char *, d, tor_free(d));
     smartlist_free(digests);
   } else {
@@ -3420,8 +3417,7 @@ int
 dirserv_remove_old_statuses(smartlist_t *fps, time_t cutoff)
 {
   int found_any = 0;
-  SMARTLIST_FOREACH(fps, char *, digest,
-  {
+  SMARTLIST_FOREACH_BEGIN(fps, char *, digest) {
     cached_dir_t *d = lookup_cached_dir_by_fp(digest);
     if (!d)
       continue;
@@ -3430,7 +3426,7 @@ dirserv_remove_old_statuses(smartlist_t *fps, time_t cutoff)
       tor_free(digest);
       SMARTLIST_DEL_CURRENT(fps, digest);
     }
-  });
+  } SMARTLIST_FOREACH_END(digest);
 
   return found_any;
 }
@@ -3469,7 +3465,7 @@ int
 dirserv_have_any_serverdesc(smartlist_t *fps, int spool_src)
 {
   time_t publish_cutoff = time(NULL)-ROUTER_MAX_AGE_TO_PUBLISH;
-  SMARTLIST_FOREACH(fps, const char *, fp, {
+  SMARTLIST_FOREACH_BEGIN(fps, const char *, fp) {
       switch (spool_src)
       {
         case DIR_SPOOL_EXTRA_BY_DIGEST:
@@ -3485,7 +3481,7 @@ dirserv_have_any_serverdesc(smartlist_t *fps, int spool_src)
             return 1;
           break;
       }
-  });
+  } SMARTLIST_FOREACH_END(fp);
   return 0;
 }
 
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index c5bd213e53..79958739a5 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -372,8 +372,7 @@ get_frequent_members(smartlist_t *out, smartlist_t *in, int min)
 {
   char *cur = NULL;
   int count = 0;
-  SMARTLIST_FOREACH(in, char *, cp,
-  {
+  SMARTLIST_FOREACH_BEGIN(in, char *, cp) {
     if (cur && !strcmp(cp, cur)) {
       ++count;
     } else {
@@ -382,7 +381,7 @@ get_frequent_members(smartlist_t *out, smartlist_t *in, int min)
       cur = cp;
       count = 1;
     }
-  });
+  } SMARTLIST_FOREACH_END(cp);
   if (count > min)
     smartlist_add(out, cur);
 }
@@ -445,8 +444,7 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method,
    * date cannot tie, we use the descriptor with the smaller digest.
    */
   smartlist_sort(votes, _compare_vote_rs);
-  SMARTLIST_FOREACH(votes, vote_routerstatus_t *, rs,
-  {
+  SMARTLIST_FOREACH_BEGIN(votes, vote_routerstatus_t *, rs) {
     if (cur && !compare_vote_rs(cur, rs)) {
       ++cur_n;
     } else {
@@ -460,7 +458,7 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method,
       cur_n = 1;
       cur = rs;
     }
-  });
+  } SMARTLIST_FOREACH_END(rs);
 
   if (cur_n > most_n ||
       (cur && cur_n == most_n && cur->status.published_on > most_published)) {
@@ -1599,12 +1597,10 @@ networkstatus_compute_consensus(smartlist_t *votes,
     chosen_named_idx = smartlist_string_pos(flags, "Named");
 
     /* Build the flag index. */
-    SMARTLIST_FOREACH(votes, networkstatus_t *, v,
-    {
+    SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, v) {
       flag_map[v_sl_idx] = tor_malloc_zero(
                            sizeof(int)*smartlist_len(v->known_flags));
-      SMARTLIST_FOREACH(v->known_flags, const char *, fl,
-      {
+      SMARTLIST_FOREACH_BEGIN(v->known_flags, const char *, fl) {
         int p = smartlist_string_pos(flags, fl);
         tor_assert(p >= 0);
         flag_map[v_sl_idx][fl_sl_idx] = p;
@@ -1613,21 +1609,21 @@ networkstatus_compute_consensus(smartlist_t *votes,
           named_flag[v_sl_idx] = fl_sl_idx;
         if (!strcmp(fl, "Unnamed"))
           unnamed_flag[v_sl_idx] = fl_sl_idx;
-      });
+      } SMARTLIST_FOREACH_END(fl);
       n_voter_flags[v_sl_idx] = smartlist_len(v->known_flags);
       size[v_sl_idx] = smartlist_len(v->routerstatus_list);
-    });
+    } SMARTLIST_FOREACH_END(v);
 
     /* Named and Unnamed get treated specially */
     if (consensus_method >= 2) {
-      SMARTLIST_FOREACH(votes, networkstatus_t *, v,
-      {
+      SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, v) {
         uint64_t nf;
         if (named_flag[v_sl_idx]<0)
           continue;
         nf = U64_LITERAL(1) << named_flag[v_sl_idx];
-        SMARTLIST_FOREACH(v->routerstatus_list, vote_routerstatus_t *, rs,
-        {
+        SMARTLIST_FOREACH_BEGIN(v->routerstatus_list,
+                                vote_routerstatus_t *, rs) {
+
           if ((rs->flags & nf) != 0) {
             const char *d = strmap_get_lc(name_to_id_map, rs->status.nickname);
             if (!d) {
@@ -1642,16 +1638,16 @@ networkstatus_compute_consensus(smartlist_t *votes,
               /* It's already a conflict, or it's already this ID. */
             }
           }
-        });
-      });
-      SMARTLIST_FOREACH(votes, networkstatus_t *, v,
-      {
+        } SMARTLIST_FOREACH_END(rs);
+      } SMARTLIST_FOREACH_END(v);
+
+      SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, v) {
         uint64_t uf;
         if (unnamed_flag[v_sl_idx]<0)
           continue;
         uf = U64_LITERAL(1) << unnamed_flag[v_sl_idx];
-        SMARTLIST_FOREACH(v->routerstatus_list, vote_routerstatus_t *, rs,
-        {
+        SMARTLIST_FOREACH_BEGIN(v->routerstatus_list,
+                                vote_routerstatus_t *, rs) {
           if ((rs->flags & uf) != 0) {
             const char *d = strmap_get_lc(name_to_id_map, rs->status.nickname);
             if (d == conflict || d == unknown) {
@@ -1666,8 +1662,8 @@ networkstatus_compute_consensus(smartlist_t *votes,
               /* It's mapped to a different name. */
             }
           }
-        });
-      });
+        } SMARTLIST_FOREACH_END(rs);
+      } SMARTLIST_FOREACH_END(v);
     }
 
     /* Now go through all the votes */
@@ -1789,8 +1785,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
 
       /* Set the flags. */
       smartlist_add(chosen_flags, (char*)"s"); /* for the start of the line. */
-      SMARTLIST_FOREACH(flags, const char *, fl,
-      {
+      SMARTLIST_FOREACH_BEGIN(flags, const char *, fl) {
         if (!strcmp(fl, "Named")) {
           if (is_named)
             smartlist_add(chosen_flags, (char*)fl);
@@ -1810,7 +1805,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
               is_bad_exit = 1;
           }
         }
-      });
+      } SMARTLIST_FOREACH_END(fl);
 
       /* Starting with consensus method 4 we do not list servers
        * that are not running in a consensus.  See Proposal 138 */
@@ -1875,7 +1870,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
          * that list previously */
         const char *chosen_exitsummary = NULL;
         smartlist_clear(exitsummaries);
-        SMARTLIST_FOREACH(matching_descs, vote_routerstatus_t *, vsr, {
+        SMARTLIST_FOREACH_BEGIN(matching_descs, vote_routerstatus_t *, vsr) {
           /* Check if the vote where this status comes from had the
            * proper descriptor */
           tor_assert(fast_memeq(rs_out.identity_digest,
@@ -1895,7 +1890,7 @@ networkstatus_compute_consensus(smartlist_t *votes,
               exitsummary_disagreement = 1;
             }
           }
-        });
+        } SMARTLIST_FOREACH_END(vsr);
 
         if (exitsummary_disagreement) {
           char id[HEX_DIGEST_LEN+1];
@@ -2747,9 +2742,8 @@ dirvote_fetch_missing_votes(void)
   smartlist_t *missing_fps = smartlist_new();
   char *resource;
 
-  SMARTLIST_FOREACH(router_get_trusted_dir_servers(),
-                    trusted_dir_server_t *, ds,
-    {
+  SMARTLIST_FOREACH_BEGIN(router_get_trusted_dir_servers(),
+                          trusted_dir_server_t *, ds) {
       if (!(ds->type & V3_DIRINFO))
         continue;
       if (!dirvote_get_vote(ds->v3_identity_digest,
@@ -2759,7 +2753,7 @@ dirvote_fetch_missing_votes(void)
                       DIGEST_LEN);
         smartlist_add(missing_fps, cp);
       }
-    });
+  } SMARTLIST_FOREACH_END(ds);
 
   if (!smartlist_len(missing_fps)) {
     smartlist_free(missing_fps);
@@ -2961,7 +2955,7 @@ dirvote_add_vote(const char *vote_body, const char **msg_out, int *status_out)
   update_consensus_router_descriptor_downloads(time(NULL), 1, vote);
 
   /* Now see whether we already have a vote from this authority. */
-  SMARTLIST_FOREACH(pending_vote_list, pending_vote_t *, v, {
+  SMARTLIST_FOREACH_BEGIN(pending_vote_list, pending_vote_t *, v) {
       if (fast_memeq(v->vote->cert->cache_info.identity_digest,
                    vote->cert->cache_info.identity_digest,
                    DIGEST_LEN)) {
@@ -2996,7 +2990,7 @@ dirvote_add_vote(const char *vote_body, const char **msg_out, int *status_out)
           goto err;
         }
       }
-  });
+  } SMARTLIST_FOREACH_END(v);
 
   pending_vote = tor_malloc_zero(sizeof(pending_vote_t));
   pending_vote->vote_body = new_cached_dir(tor_strndup(vote_body,
@@ -3180,8 +3174,7 @@ dirvote_compute_consensuses(void)
     int n_sigs = 0;
     /* we may have gotten signatures for this consensus before we built
      * it ourself.  Add them now. */
-    SMARTLIST_FOREACH(pending_consensus_signature_list, char *, sig,
-      {
+    SMARTLIST_FOREACH_BEGIN(pending_consensus_signature_list, char *, sig) {
         const char *msg = NULL;
         int r = dirvote_add_signatures_to_all_pending_consensuses(sig,
                                                      "pending", &msg);
@@ -3192,7 +3185,7 @@ dirvote_compute_consensuses(void)
                    "Could not add queued signature to new consensus: %s",
                    msg);
         tor_free(sig);
-      });
+    } SMARTLIST_FOREACH_END(sig);
     if (n_sigs)
       log_notice(LD_DIR, "Added %d pending signatures while building "
                  "consensus.", n_sigs);
diff --git a/src/or/geoip.c b/src/or/geoip.c
index 5d6488817a..6b7cc82b82 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -906,7 +906,7 @@ geoip_get_request_history(geoip_client_action_t action)
     return NULL;
 
   entries = smartlist_new();
-  SMARTLIST_FOREACH(geoip_countries, geoip_country_t *, c, {
+  SMARTLIST_FOREACH_BEGIN(geoip_countries, geoip_country_t *, c) {
       uint32_t tot = 0;
       c_hist_t *ent;
       tot = (action == GEOIP_CLIENT_NETWORKSTATUS) ?
@@ -917,7 +917,7 @@ geoip_get_request_history(geoip_client_action_t action)
       strlcpy(ent->country, c->countrycode, sizeof(ent->country));
       ent->total = round_to_next_multiple_of(tot, granularity);
       smartlist_add(entries, ent);
-  });
+  } SMARTLIST_FOREACH_END(c);
   smartlist_sort(entries, _c_hist_compare);
 
   strings = smartlist_new();
diff --git a/src/or/main.c b/src/or/main.c
index 95ac8d616e..20a1e086a4 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -2105,8 +2105,7 @@ dumpstats(int severity)
 
   log(severity, LD_GENERAL, "Dumping stats:");
 
-  SMARTLIST_FOREACH(connection_array, connection_t *, conn,
-  {
+  SMARTLIST_FOREACH_BEGIN(connection_array, connection_t *, conn) {
     int i = conn_sl_idx;
     log(severity, LD_GENERAL,
         "Conn %d (socket %d) type %d (%s), state %d (%s), created %d secs ago",
@@ -2144,7 +2143,7 @@ dumpstats(int severity)
     }
     circuit_dump_by_conn(conn, severity); /* dump info about all the circuits
                                            * using this conn */
-  });
+  } SMARTLIST_FOREACH_END(conn);
   log(severity, LD_NET,
       "Cells processed: "U64_FORMAT" padding\n"
       "                 "U64_FORMAT" create\n"
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index 745176de69..fadaf90da4 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -128,12 +128,12 @@ networkstatus_reset_download_failures(void)
 {
   int i;
   const smartlist_t *networkstatus_v2_list = networkstatus_get_v2_list();
-  SMARTLIST_FOREACH(networkstatus_v2_list, networkstatus_v2_t *, ns,
-     SMARTLIST_FOREACH(ns->entries, routerstatus_t *, rs,
-       {
+  SMARTLIST_FOREACH_BEGIN(networkstatus_v2_list, networkstatus_v2_t *, ns) {
+    SMARTLIST_FOREACH_BEGIN(ns->entries, routerstatus_t *, rs) {
          if (!router_get_by_descriptor_digest(rs->descriptor_digest))
            rs->need_to_mirror = 1;
-       }));;
+    } SMARTLIST_FOREACH_END(rs);
+  } SMARTLIST_FOREACH_END(ns);
 
   for (i=0; i < N_CONSENSUS_FLAVORS; ++i)
     download_status_reset(&consensus_dl_status[i]);
@@ -177,7 +177,7 @@ router_reload_v2_networkstatus(void)
     return 0;
   }
   tor_free(filename);
-  SMARTLIST_FOREACH(entries, const char *, fn, {
+  SMARTLIST_FOREACH_BEGIN(entries, const char *, fn) {
       char buf[DIGEST_LEN];
       if (maybe_delete) {
         filename = get_datadir_fname2("cached-status", fn);
@@ -201,7 +201,7 @@ router_reload_v2_networkstatus(void)
         tor_free(s);
       }
       tor_free(filename);
-    });
+  } SMARTLIST_FOREACH_END(fn);
   SMARTLIST_FOREACH(entries, char *, fn, tor_free(fn));
   smartlist_free(entries);
   networkstatus_v2_list_clean(time(NULL));
@@ -881,8 +881,7 @@ router_set_networkstatus_v2(const char *s, time_t arrived_at,
 
   {
     time_t live_until = ns->published_on + V2_NETWORKSTATUS_ROUTER_LIFETIME;
-    SMARTLIST_FOREACH(ns->entries, routerstatus_t *, rs,
-    {
+    SMARTLIST_FOREACH_BEGIN(ns->entries, routerstatus_t *, rs) {
       signed_descriptor_t *sd =
         router_get_by_descriptor_digest(rs->descriptor_digest);
       if (sd) {
@@ -891,7 +890,7 @@ router_set_networkstatus_v2(const char *s, time_t arrived_at,
       } else {
         rs->need_to_mirror = 1;
       }
-    });
+    } SMARTLIST_FOREACH_END(rs);
   }
 
   log_info(LD_DIR, "Setting networkstatus %s %s (published %s)",
@@ -2014,9 +2013,8 @@ routerstatus_list_update_named_server_map(void)
   named_server_map = strmap_new();
   strmap_free(unnamed_server_map, NULL);
   unnamed_server_map = strmap_new();
-  SMARTLIST_FOREACH(current_consensus->routerstatus_list,
-                                                   const routerstatus_t *, rs,
-    {
+  SMARTLIST_FOREACH_BEGIN(current_consensus->routerstatus_list,
+                          const routerstatus_t *, rs) {
       if (rs->is_named) {
         strmap_set_lc(named_server_map, rs->nickname,
                       tor_memdup(rs->identity_digest, DIGEST_LEN));
@@ -2024,7 +2022,7 @@ routerstatus_list_update_named_server_map(void)
       if (rs->is_unnamed) {
         strmap_set_lc(unnamed_server_map, rs->nickname, (void*)1);
       }
-    });
+  } SMARTLIST_FOREACH_END(rs);
 }
 
 /** Given a list <b>routers</b> of routerinfo_t *, update each status field
@@ -2082,7 +2080,7 @@ routers_update_status_from_consensus_networkstatus(smartlist_t *routers,
   } SMARTLIST_FOREACH_JOIN_END(rs, router);
 
   /* Now update last_listed_as_valid_until from v2 networkstatuses. */
-  SMARTLIST_FOREACH(networkstatus_v2_list, networkstatus_v2_t *, ns, {
+  SMARTLIST_FOREACH_BEGIN(networkstatus_v2_list, networkstatus_v2_t *, ns) {
     time_t live_until = ns->published_on + V2_NETWORKSTATUS_ROUTER_LIFETIME;
     SMARTLIST_FOREACH_JOIN(ns->entries, const routerstatus_t *, rs,
                          routers, routerinfo_t *, ri,
@@ -2095,7 +2093,7 @@ routers_update_status_from_consensus_networkstatus(smartlist_t *routers,
           ri->cache_info.last_listed_as_valid_until = live_until;
       }
     } SMARTLIST_FOREACH_JOIN_END(rs, ri);
-  });
+  } SMARTLIST_FOREACH_END(ns);
 
   router_dir_info_changed();
 }
@@ -2162,7 +2160,7 @@ networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now)
   }
 
   statuses = smartlist_new();
-  SMARTLIST_FOREACH(rl->routers, routerinfo_t *, ri, {
+  SMARTLIST_FOREACH_BEGIN(rl->routers, routerinfo_t *, ri) {
     node_t *node = node_get_mutable_by_id(ri->cache_info.identity_digest);
     if (!node)
       continue;
@@ -2175,7 +2173,7 @@ networkstatus_getinfo_by_purpose(const char *purpose_string, time_t now)
     /* then generate and write out status lines for each of them */
     set_routerstatus_from_routerinfo(&rs, node, ri, now, 0, 0, 0, 0);
     smartlist_add(statuses, networkstatus_getinfo_helper_single(&rs));
-  });
+  } SMARTLIST_FOREACH_END(ri);
 
   answer = smartlist_join_strings(statuses, "", 0, NULL);
   SMARTLIST_FOREACH(statuses, char *, cp, tor_free(cp));
diff --git a/src/or/policies.c b/src/or/policies.c
index b2b962dfd7..3018803bc4 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -77,8 +77,7 @@ policy_expand_private(smartlist_t **policy)
 
   tmp = smartlist_new();
 
-  SMARTLIST_FOREACH(*policy, addr_policy_t *, p,
-  {
+  SMARTLIST_FOREACH_BEGIN(*policy, addr_policy_t *, p) {
      if (! p->is_private) {
        smartlist_add(tmp, p);
        continue;
@@ -95,7 +94,7 @@ policy_expand_private(smartlist_t **policy)
        smartlist_add(tmp, addr_policy_get_canonical_entry(&newpolicy));
      }
      addr_policy_free(p);
-  });
+  } SMARTLIST_FOREACH_END(p);
 
   smartlist_free(*policy);
   *policy = tmp;
@@ -127,8 +126,7 @@ parse_addr_policy(config_line_t *cfg, smartlist_t **dest,
   for (; cfg; cfg = cfg->next) {
     smartlist_split_string(entries, cfg->value, ",",
                            SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-    SMARTLIST_FOREACH(entries, const char *, ent,
-    {
+    SMARTLIST_FOREACH_BEGIN(entries, const char *, ent) {
       log_debug(LD_CONFIG,"Adding new entry '%s'",ent);
       item = router_parse_addr_policy_item_from_string(ent, assume_action);
       if (item) {
@@ -137,7 +135,7 @@ parse_addr_policy(config_line_t *cfg, smartlist_t **dest,
         log_warn(LD_CONFIG,"Malformed policy '%s'.", ent);
         r = -1;
       }
-    });
+    } SMARTLIST_FOREACH_END(ent);
     SMARTLIST_FOREACH(entries, char *, ent, tor_free(ent));
     smartlist_clear(entries);
   }
@@ -912,7 +910,7 @@ exit_policy_is_general_exit_helper(smartlist_t *policy, int port)
   char subnet_status[256];
 
   memset(subnet_status, 0, sizeof(subnet_status));
-  SMARTLIST_FOREACH(policy, addr_policy_t *, p, {
+  SMARTLIST_FOREACH_BEGIN(policy, addr_policy_t *, p) {
     if (tor_addr_family(&p->addr) != AF_INET)
       continue; /* IPv4 only for now */
     if (p->prt_min > port || p->prt_max < port)
@@ -943,7 +941,7 @@ exit_policy_is_general_exit_helper(smartlist_t *policy, int port)
         subnet_status[i] = 1;
       }
     }
-  });
+  } SMARTLIST_FOREACH_END(p);
   return 0;
 }
 
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 6a51874699..6af4778dfc 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -500,16 +500,16 @@ rend_config_services(const or_options_t *options, int validate_only)
     /* Copy introduction points to new services. */
     /* XXXX This is O(n^2), but it's only called on reconfigure, so it's
      * probably ok? */
-    SMARTLIST_FOREACH(rend_service_list, rend_service_t *, new, {
-      SMARTLIST_FOREACH(old_service_list, rend_service_t *, old, {
+    SMARTLIST_FOREACH_BEGIN(rend_service_list, rend_service_t *, new) {
+      SMARTLIST_FOREACH_BEGIN(old_service_list, rend_service_t *, old) {
         if (!strcmp(old->directory, new->directory)) {
           smartlist_add_all(new->intro_nodes, old->intro_nodes);
           smartlist_clear(old->intro_nodes);
           smartlist_add(surviving_services, old);
           break;
         }
-      });
-    });
+      } SMARTLIST_FOREACH_END(old);
+    } SMARTLIST_FOREACH_END(new);
 
     /* Close introduction circuits of services we don't serve anymore. */
     /* XXXX it would be nicer if we had a nicer abstraction to use here,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 8349b7ad6c..de1a66ce16 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -731,8 +731,7 @@ router_rebuild_store(int flags, desc_store_t *store)
   smartlist_sort(signed_descriptors, _compare_signed_descriptors_by_age);
 
   /* Now, add the appropriate members to chunk_list */
-  SMARTLIST_FOREACH(signed_descriptors, signed_descriptor_t *, sd,
-    {
+  SMARTLIST_FOREACH_BEGIN(signed_descriptors, signed_descriptor_t *, sd) {
       sized_chunk_t *c;
       const char *body = signed_descriptor_get_body_impl(sd, 1);
       if (!body) {
@@ -748,7 +747,7 @@ router_rebuild_store(int flags, desc_store_t *store)
       c->len = sd->signed_descriptor_len + sd->annotations_len;
       total_expected_len += c->len;
       smartlist_add(chunk_list, c);
-    });
+  } SMARTLIST_FOREACH_END(sd);
 
   if (write_chunks_to_file(fname_tmp, chunk_list, 1)<0) {
     log_warn(LD_FS, "Error writing router store to disk.");
@@ -787,8 +786,7 @@ router_rebuild_store(int flags, desc_store_t *store)
   log_info(LD_DIR, "Reconstructing pointers into cache");
 
   offset = 0;
-  SMARTLIST_FOREACH(signed_descriptors, signed_descriptor_t *, sd,
-    {
+  SMARTLIST_FOREACH_BEGIN(signed_descriptors, signed_descriptor_t *, sd) {
       if (sd->do_not_cache)
         continue;
       sd->saved_location = SAVED_IN_CACHE;
@@ -798,7 +796,7 @@ router_rebuild_store(int flags, desc_store_t *store)
       }
       offset += sd->signed_descriptor_len + sd->annotations_len;
       signed_descriptor_get_body(sd); /* reconstruct and assert */
-    });
+  } SMARTLIST_FOREACH_END(sd);
 
   tor_free(fname);
   fname = get_datadir_fname_suffix(store->fname_base, ".new");
@@ -1329,8 +1327,7 @@ mark_all_trusteddirservers_up(void)
          node->is_running = 1;
     });
   if (trusted_dir_servers) {
-    SMARTLIST_FOREACH(trusted_dir_servers, trusted_dir_server_t *, dir,
-    {
+    SMARTLIST_FOREACH_BEGIN(trusted_dir_servers, trusted_dir_server_t *, dir) {
       routerstatus_t *rs;
       dir->is_running = 1;
       download_status_reset(&dir->v2_ns_dl_status);
@@ -1339,7 +1336,7 @@ mark_all_trusteddirservers_up(void)
         rs->last_dir_503_at = 0;
         control_event_networkstatus_changed_single(rs);
       }
-    });
+    } SMARTLIST_FOREACH_END(dir);
   }
   router_dir_info_changed();
 }
@@ -3676,8 +3673,7 @@ routerlist_remove_old_routers(void)
   cutoff = now - OLD_ROUTER_DESC_MAX_AGE;
   /* Build a list of all the descriptors that _anybody_ lists. */
   if (caches && networkstatus_v2_list) {
-    SMARTLIST_FOREACH(networkstatus_v2_list, networkstatus_v2_t *, ns,
-    {
+    SMARTLIST_FOREACH_BEGIN(networkstatus_v2_list, networkstatus_v2_t *, ns) {
       /* XXXX The inner loop here gets pretty expensive, and actually shows up
        * on some profiles.  It may be the reason digestmap_set shows up in
        * profiles too.  If instead we kept a per-descriptor digest count of
@@ -3686,10 +3682,11 @@ routerlist_remove_old_routers(void)
        * improvement, possibly 1-4% if it also removes digestmap_set from the
        * profile.  Not worth it for 0.1.2.x, though.  The new directory
        * system will obsolete this whole thing in 0.2.0.x. */
-      SMARTLIST_FOREACH(ns->entries, routerstatus_t *, rs,
+      SMARTLIST_FOREACH_BEGIN(ns->entries, routerstatus_t *, rs) {
         if (rs->published_on >= cutoff)
-          digestset_add(retain, rs->descriptor_digest));
-    });
+          digestset_add(retain, rs->descriptor_digest);
+      } SMARTLIST_FOREACH_END(rs);
+    } SMARTLIST_FOREACH_END(ns);
   }
 
   /* Retain anything listed in the consensus. */
@@ -3979,7 +3976,7 @@ router_load_extrainfo_from_string(const char *s, const char *eos,
 
   log_info(LD_DIR, "%d elements to add", smartlist_len(extrainfo_list));
 
-  SMARTLIST_FOREACH(extrainfo_list, extrainfo_t *, ei, {
+  SMARTLIST_FOREACH_BEGIN(extrainfo_list, extrainfo_t *, ei) {
       was_router_added_t added =
         router_add_extrainfo_to_routerlist(ei, &msg, from_cache, !from_cache);
       if (WRA_WAS_ADDED(added) && requested_fingerprints) {
@@ -3994,7 +3991,7 @@ router_load_extrainfo_from_string(const char *s, const char *eos,
          * all the extrainfos we want, and we never actually act on them
          * inside Tor, this should be harmless. */
       }
-    });
+  } SMARTLIST_FOREACH_END(ei);
 
   routerlist_assert_ok(routerlist);
   router_rebuild_store(0, &router_get_routerlist()->extrainfo_store);
@@ -4524,8 +4521,7 @@ update_router_descriptor_cache_downloads_v2(time_t now)
    * descriptor from the corresponding authority.
    */
   n_download = 0;
-  SMARTLIST_FOREACH(networkstatus_v2_list, networkstatus_v2_t *, ns,
-    {
+  SMARTLIST_FOREACH_BEGIN(networkstatus_v2_list, networkstatus_v2_t *, ns) {
       trusted_dir_server_t *ds;
       smartlist_t *dl;
       dl = downloadable[ns_sl_idx] = smartlist_new();
@@ -4547,8 +4543,7 @@ update_router_descriptor_cache_downloads_v2(time_t now)
       if (ds && !ds->is_running)
         continue;
 
-      SMARTLIST_FOREACH(ns->entries, routerstatus_t * , rs,
-        {
+      SMARTLIST_FOREACH_BEGIN(ns->entries, routerstatus_t * , rs) {
           if (!rs->need_to_mirror)
             continue;
           if (router_get_by_descriptor_digest(rs->descriptor_digest)) {
@@ -4569,8 +4564,8 @@ update_router_descriptor_cache_downloads_v2(time_t now)
             smartlist_add(dl, rs->descriptor_digest);
             ++n_download;
           }
-        });
-    });
+      } SMARTLIST_FOREACH_END(rs);
+  } SMARTLIST_FOREACH_END(ns);
 
   /* At random, assign descriptors to authorities such that:
    * - if d is a member of some downloadable[x], d is a member of some
@@ -4727,7 +4722,7 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote,
     log_info(LD_DIR, "%d router descriptors listed in consensus are "
              "currently in old_routers; making them current.",
              smartlist_len(no_longer_old));
-    SMARTLIST_FOREACH(no_longer_old, signed_descriptor_t *, sd, {
+    SMARTLIST_FOREACH_BEGIN(no_longer_old, signed_descriptor_t *, sd) {
         const char *msg;
         was_router_added_t r;
         routerinfo_t *ri = routerlist_reparse_old(rl, sd);
@@ -4740,7 +4735,7 @@ update_consensus_router_descriptor_downloads(time_t now, int is_vote,
           log_warn(LD_DIR, "Couldn't add re-parsed router: %s",
                    msg?msg:"???");
         }
-      });
+    } SMARTLIST_FOREACH_END(sd);
     routerlist_assert_ok(rl);
   }
 
@@ -5294,8 +5289,7 @@ routerlist_assert_ok(const routerlist_t *rl)
   signed_descriptor_t *sd2;
   if (!rl)
     return;
-  SMARTLIST_FOREACH(rl->routers, routerinfo_t *, r,
-  {
+  SMARTLIST_FOREACH_BEGIN(rl->routers, routerinfo_t *, r) {
     r2 = rimap_get(rl->identity_map, r->cache_info.identity_digest);
     tor_assert(r == r2);
     sd2 = sdmap_get(rl->desc_digest_map,
@@ -5325,9 +5319,8 @@ routerlist_assert_ok(const routerlist_t *rl)
       tor_assert(sd3 == &(r->cache_info));
     }
     */
-  });
-  SMARTLIST_FOREACH(rl->old_routers, signed_descriptor_t *, sd,
-  {
+  } SMARTLIST_FOREACH_END(r);
+  SMARTLIST_FOREACH_BEGIN(rl->old_routers, signed_descriptor_t *, sd) {
     r2 = rimap_get(rl->identity_map, sd->identity_digest);
     tor_assert(sd != &(r2->cache_info));
     sd2 = sdmap_get(rl->desc_digest_map, sd->signed_descriptor_digest);
@@ -5340,7 +5333,7 @@ routerlist_assert_ok(const routerlist_t *rl)
       tor_assert(sd3 == sd);
     }
     */
-  });
+  } SMARTLIST_FOREACH_END(sd);
 
   RIMAP_FOREACH(rl->identity_map, d, r) {
     tor_assert(tor_memeq(r->cache_info.identity_digest, d, DIGEST_LEN));
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 37609bddc1..4231a17c67 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -775,7 +775,7 @@ tor_version_is_obsolete(const char *myversion, const char *versionlist)
     goto done;
   }
 
-  SMARTLIST_FOREACH(version_sl, const char *, cp, {
+  SMARTLIST_FOREACH_BEGIN(version_sl, const char *, cp) {
     if (!strcmpstart(cp, "Tor "))
       cp += 4;
 
@@ -797,7 +797,7 @@ tor_version_is_obsolete(const char *myversion, const char *versionlist)
         found_older = 1;
       }
     }
-  });
+  } SMARTLIST_FOREACH_END(cp);
 
   /* We didn't find the listed version. Is it new or old? */
   if (found_any_in_series && !found_newer_in_series && found_newer) {
diff --git a/src/test/test.c b/src/test/test.c
index 454fc54d52..6bf2d28d90 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -131,8 +131,7 @@ rm_rf(const char *dir)
 
   elements = tor_listdir(dir);
   if (elements) {
-    SMARTLIST_FOREACH(elements, const char *, cp,
-       {
+    SMARTLIST_FOREACH_BEGIN(elements, const char *, cp) {
          char *tmp = NULL;
          tor_asprintf(&tmp, "%s"PATH_SEPARATOR"%s", dir, cp);
          if (0 == stat(tmp,&st) && (st.st_mode & S_IFDIR)) {
@@ -143,7 +142,7 @@ rm_rf(const char *dir)
            }
          }
          tor_free(tmp);
-       });
+    } SMARTLIST_FOREACH_END(cp);
     SMARTLIST_FOREACH(elements, char *, cp, tor_free(cp));
     smartlist_free(elements);
   }
diff --git a/src/test/test_util.c b/src/test/test_util.c
index 632ef68bd6..9eca90492d 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -2604,8 +2604,7 @@ test_util_split_lines(void *ptr)
     j = 0;
     log_info(LD_GENERAL, "Splitting test %d of length %d",
              i, tests[i].orig_length);
-    SMARTLIST_FOREACH(sl, const char *, line,
-    {
+    SMARTLIST_FOREACH_BEGIN(sl, const char *, line) {
       /* Check we have not got too many lines */
       test_assert(j < MAX_SPLIT_LINE_COUNT);
       /* Check that there actually should be a line here */
@@ -2615,7 +2614,7 @@ test_util_split_lines(void *ptr)
       /* Check that the line is as expected */
       test_streq(line, tests[i].split_line[j]);
       j++;
-    });
+    } SMARTLIST_FOREACH_END(line);
     /* Check that we didn't miss some lines */
     test_eq_ptr(NULL, tests[i].split_line[j]);
     tor_free(orig_line);