Change all SMARTLIST_FOREACH loops of >=10 lines to use BEGIN/END

The SMARTLIST_FOREACH macro is more convenient than BEGIN/END when
you have a nice short loop body, but using it for long bodies makes
your preprocessor tell the compiler that all the code is on the same
line.  That causes grief, since compiler warnings and debugger lines
will all refer to that one line.

So, here's a new style rule: SMARTLIST_FOREACH blocks need to be
short.

19 files changed, 139 insertions, 176 deletions

diff --git a/changes/smartlist_foreach b/changes/smartlist_foreach
new file mode 100644
index 0000000000..d1e3505468
--- /dev/null
+++ b/changes/smartlist_foreach
@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Do not allow the body of any SMARTLIST_FOREACH block to exceed
+      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.
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 f5b5c8fb58..4e9518f1d2 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..1979250c5b 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -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/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);