From 78dec943074c26747abc7a68cd6aec5269100569 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Jul 2012 10:12:19 -0400 Subject: [PATCH] Tweaks to 6400 changes file and docs as suggested by arma --- changes/smartlist_foreach | 10 ++++++---- src/common/container.h | 24 +++++++++++++++--------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/changes/smartlist_foreach b/changes/smartlist_foreach index 0a6c960427..2fd3a1a85c 100644 --- a/changes/smartlist_foreach +++ b/changes/smartlist_foreach @@ -1,6 +1,8 @@ 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. + - 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. - - Do not allow SMARTLIST_FOREACH blocks to nest. Any nested block - ought to be using SMARTLIST_FOREACH_{BEGIN,END}. + Issue 6400. + - Do not nest SMARTLIST_FOREACH blocks within one another. Any + nested block ought to be using SMARTLIST_FOREACH_{BEGIN,END}. + Issue 6400. diff --git a/src/common/container.h b/src/common/container.h index c11c2eb82f..dab3b83f37 100644 --- a/src/common/container.h +++ b/src/common/container.h @@ -142,9 +142,9 @@ char *smartlist_join_strings2(smartlist_t *sl, const char *join, /** Iterate over the items in a smartlist sl, in order. For each item, * assign it to a new local variable of type type named var, and - * execute the statement cmd. Inside the loop, the loop index can - * be accessed as var_sl_idx and the length of the list can be accessed - * as var_sl_len. + * execute the statements inside the loop body. Inside the loop, the loop + * index can be accessed as var_sl_idx and the length of the list can + * be accessed as var_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: *
  *   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);
  * 
* * Example use (advanced): *
- *   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);
  * 
*/ /* 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 + * cmd 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; \