Merge branch 'maint-0.2.3' into release-0.2.3

33 files changed, 230 insertions, 209 deletions

diff --git a/changes/bug6274 b/changes/bug6274
new file mode 100644
index 0000000000..ad1abcde54
--- /dev/null
+++ b/changes/bug6274
@@ -0,0 +1,3 @@
+ o Minor bugfixes:
+   - Ignore ServerTransportPlugin lines when Tor is not configured as
+     a relay. Fixes bug 6274; bugfix on 0.2.3.6-alpha.
diff --git a/changes/bug6274_2 b/changes/bug6274_2
new file mode 100644
index 0000000000..89576f9328
--- /dev/null
+++ b/changes/bug6274_2
@@ -0,0 +1,3 @@
+ o Minor features:
+   - Terminate active server managed proxies if Tor stops being a
+     relay. Addresses parts of bug 6274; bugfix on 0.2.3.6-alpha.
diff --git a/changes/bug6377 b/changes/bug6377
new file mode 100644
index 0000000000..a3a3672783
--- /dev/null
+++ b/changes/bug6377
@@ -0,0 +1,4 @@
+ o Testing:
+   - Make it possible to set the TestingTorNetwork configuration
+     option using AlternateDirAuthority and AlternateBridgeAuthority
+     as an alternative to setting DirServer.
diff --git a/changes/bug6387 b/changes/bug6387
new file mode 100644
index 0000000000..73fc4f7cfe
--- /dev/null
+++ b/changes/bug6387
@@ -0,0 +1,3 @@
+  o Documentation:
+    - Clarify the documentation for the Alternate*Authority options.
+      Fixes bug 6387.
diff --git a/changes/bug6397 b/changes/bug6397
new file mode 100644
index 0000000000..23d8359bd2
--- /dev/null
+++ b/changes/bug6397
@@ -0,0 +1,4 @@
+  o Major bugfixes:
+    - When disabling guards for having too high a proportion of failed
+      circuits, make sure to look at each guard. Fix for bug 6397; bugfix
+      on 0.2.3.17-beta.
diff --git a/changes/bug6436 b/changes/bug6436
new file mode 100644
index 0000000000..2c163df105
--- /dev/null
+++ b/changes/bug6436
@@ -0,0 +1,3 @@
+  o Minor features:
+    - Provide a better error message about possible OSX Asciidoc failure
+      reasons. Fix for bug 6436.
diff --git a/changes/cov709056 b/changes/cov709056
new file mode 100644
index 0000000000..64a75ad8a2
--- /dev/null
+++ b/changes/cov709056
@@ -0,0 +1,4 @@
+  o Minor bugfixes:
+    - Check return value of fputs() when writing authority certificate
+      file. Fixes Coverity issue 709056; bugfix on 0.2.0.1-alpha.
+
diff --git a/changes/port_doc b/changes/port_doc
new file mode 100644
index 0000000000..0e8662f0ab
--- /dev/null
+++ b/changes/port_doc
@@ -0,0 +1,3 @@
+  o Minor features (usability):
+    - Try to make the warning when giving an obsolete SOCKSListenAddress
+      a littel more useful.
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/doc/asciidoc-helper.sh b/doc/asciidoc-helper.sh
index 33e1360a71..dd420f7c4f 100755
--- a/doc/asciidoc-helper.sh
+++ b/doc/asciidoc-helper.sh
@@ -54,7 +54,9 @@ You need a working asciidoc installed to be able to build the manpage.
 a2x is installed, but for some reason it isn't working.  Sometimes
 This happens because required docbook support files are missing.
 Please install docbook-xsl, docbook-xml, and libxml2-utils (Debian) or
-similar.
+similar. If you use homebrew on Mac OS X, install the docbook formula
+and add "export XML_CATALOG_FILES=/usr/local/etc/xml/catalog" to your
+.bashrc
 
 Alternatively, to build without manpages, use the --disable-asciidoc
 argument when calling configure.
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index a4a0ed7da9..78c34874c5 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -319,11 +319,14 @@ GENERAL OPTIONS
 **AlternateHSAuthority** [__nickname__] [**flags**] __address__:__port__ __fingerprint__ +
 
 **AlternateBridgeAuthority** [__nickname__] [**flags**] __address__:__port__ __ fingerprint__::
-    As DirServer, but replaces less of the default directory authorities. Using
+    These options behave as DirServer, but they replace fewer of the
+    default directory authorities. Using
     AlternateDirAuthority replaces the default Tor directory authorities, but
-    leaves the hidden service authorities and bridge authorities in place.
-    Similarly, Using AlternateHSAuthority replaces the default hidden service
-    authorities, but not the directory or bridge authorities.
+    leaves the default hidden service authorities and bridge authorities in
+    place.  Similarly, AlternateHSAuthority replaces the default hidden
+    service authorities, but not the directory or bridge authorities; and
+    AlternateBridgeAuthority replaces the default bridge authority,
+    but leaves the directory and hidden service authorities alone.
 
 **DisableAllSwap** **0**|**1**::
     If set to 1, Tor will attempt to lock all current and future memory pages,
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 6d1e4e7643..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 "
@@ -4728,8 +4728,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
     }
   }
 
-  SMARTLIST_FOREACH(new_entry_guards, entry_guard_t *, e,
-   {
+  SMARTLIST_FOREACH_BEGIN(new_entry_guards, entry_guard_t *, e) {
      char *sp;
      char *val = digestmap_get(added_by, e->identity);
      if (val && (sp = strchr(val, ' '))) {
@@ -4747,9 +4746,10 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
          e->chosen_on_date = time(NULL) - crypto_rand_int(3600*24*30);
        }
      }
-     if (node->path_bias_disabled && !node->bad_since)
-       node->bad_since = time(NULL);
-   });
+     if (e->path_bias_disabled && !e->bad_since)
+       e->bad_since = time(NULL);
+    }
+  SMARTLIST_FOREACH_END(e);
 
   if (*msg || !set) {
     SMARTLIST_FOREACH(new_entry_guards, entry_guard_t *, e,
@@ -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 d90e0fc996..918efd4cf8 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1464,7 +1464,7 @@ options_act(const or_options_t *old_options)
     }
   }
 
-  if (options->ServerTransportPlugin) {
+  if (options->ServerTransportPlugin && server_mode(options)) {
     for (cl = options->ServerTransportPlugin; cl; cl = cl->next) {
       if (parse_server_transport_line(cl->value, 0)<0) {
         log_warn(LD_BUG,
@@ -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 ||
@@ -4019,6 +4019,13 @@ options_validate(or_options_t *old_options, or_options_t *options,
       REJECT("Server transport line did not parse. See logs for details.");
   }
 
+  if (options->ServerTransportPlugin && !server_mode(options)) {
+    log_notice(LD_GENERAL, "Tor is not configured as a relay but you specified"
+               " a ServerTransportPlugin line (%s). The ServerTransportPlugin "
+               "line will be ignored.",
+               esc_for_log(options->ServerTransportPlugin->value));
+  }
+
   if (options->ConstrainedSockets) {
     /* If the user wants to constrain socket buffer use, make sure the desired
      * limit is between MIN|MAX_TCPSOCK_BUFFER in k increments. */
@@ -4091,9 +4098,13 @@ options_validate(or_options_t *old_options, or_options_t *options,
     });
   }
 
-  if (options->TestingTorNetwork && !options->DirServers) {
+  if (options->TestingTorNetwork &&
+      !(options->DirServers ||
+        (options->AlternateDirAuthority &&
+         options->AlternateBridgeAuthority))) {
     REJECT("TestingTorNetwork may only be configured in combination with "
-           "a non-default set of DirServers.");
+           "a non-default set of DirServer or both of AlternateDirAuthority "
+           "and AlternateBridgeAuthority configured.");
   }
 
   if (options->AllowSingleHopExits && !options->DirServers) {
@@ -5638,7 +5649,7 @@ parse_port_config(smartlist_t *out,
         mainport = (int)tor_parse_long(ports->value, 10, 0, 65535, &ok, NULL);
         if (!ok) {
           log_warn(LD_CONFIG, "%sListenAddress can only be used with a single "
-                   "%sPort with value \"auto\" or 1-65535.",
+                   "%sPort with value \"auto\" or 1-65535 and no options set.",
                    portname, portname);
           return -1;
         }
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/dns.c b/src/or/dns.c
index 02447ece81..da11668c6d 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -567,7 +567,7 @@ send_resolved_hostname_cell(edge_connection_t *conn, const char *hostname)
   // log_notice(LD_EXIT, "Sent");
 }
 
-/** See if we have a cache entry for <b>exitconn</b>-\>address. if so,
+/** See if we have a cache entry for <b>exitconn</b>-\>address. If so,
  * if resolve valid, put it into <b>exitconn</b>-\>addr and return 1.
  * If resolve failed, free exitconn and return -1.
  *
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);
diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c
index a8ffc07796..38b2101a3c 100644
--- a/src/tools/tor-gencert.c
+++ b/src/tools/tor-gencert.c
@@ -497,7 +497,12 @@ generate_certificate(void)
     return 1;
   }
 
-  fputs(buf, f);
+  if (fputs(buf, f) < 0) {
+    log_err(LD_GENERAL, "Couldn't write to %s: %s",
+            certificate_file, strerror(errno));
+    fclose(f);
+    return 1;
+  }
   fclose(f);
   return 0;
 }