Commit 7faf115d authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

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.
parent 21c6c848

changes/smartlist_foreach

0 → 100644
+4 −0
Original line number Diff line number Diff line 
  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.

src/common/log.c

+2 −3
Original line number Diff line number Diff line
@@ -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)

src/or/circuitbuild.c

+6 −8
Original line number Diff line number Diff line
@@ -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;

}

src/or/config.c

+4 −4
Original line number Diff line number Diff line
@@ -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 ||

src/or/connection.c

+6 −9
Original line number Diff line number Diff line
@@ -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];