r12771@catbus: nickm | 2007-05-16 18:12:32 -0400

 Make -Wstrict-overflow=5 happy with GCC 4.2.  It is kind of a pain, but it does agood job of letting us know where we can make our code better by simplifying dependent conditionals.


svn:r10201

configure.in

@@ -631,8 +631,7 @@ if test x$enable_gcc_warnings = xyes; then
     # These warnings break gcc 4.0.2 and work on gcc 4.2
     # XXXX020 Use -fstack-protector.
     # XXXX020 See if any of these work with earlier versions.
-    # XXXX020 See if we can get -Wstrict-overflow=x for x>1 working.
-    CFLAGS="$CFLAGS -Waddress -Wmissing-noreturn -Wnormalized=id -Woverride-init -W"
+    CFLAGS="$CFLAGS -Waddress -Wmissing-noreturn -Wnormalized=id -Woverride-init -Wstrict-overflow=5"
   fi
 
 ##This will break the world on some 64-bit architectures

src/or/policies.c

@@ -508,29 +508,30 @@ exit_policy_remove_redundancies(addr_policy_t **dest)
   previous = NULL;
   while (ap) {
     for (tmp=ap->next; tmp; tmp=tmp->next) {
-      if (ap->policy_type != tmp->policy_type &&
-          addr_policy_intersects(ap, tmp)) {
-        tmp = NULL; /* so that we advance previous and ap */
-        break;
-      }
-      if (ap->policy_type == tmp->policy_type &&
-          addr_policy_covers(tmp, ap)) {
-        log(LOG_DEBUG, LD_CONFIG, "Removing exit policy %s.  It is already "
-            "covered by %s.", ap->string, tmp->string);
-        victim = ap;
-        ap = ap->next;
-
-        if (previous) {
-          assert(previous->next == victim);
-          previous->next = victim->next;
-        } else {
-          assert(*dest == victim);
-          *dest = victim->next;
+      if (ap->policy_type != tmp->policy_type) {
+        if (addr_policy_intersects(ap, tmp)) {
+          tmp = NULL; /* so that we advance previous and ap */
+          break;
+        }
+      } else { /* policy_types are equal. */
+        if (addr_policy_covers(tmp, ap)) {
+          log(LOG_DEBUG, LD_CONFIG, "Removing exit policy %s.  It is already "
+              "covered by %s.", ap->string, tmp->string);
+          victim = ap;
+          ap = ap->next;
+
+          if (previous) {
+            assert(previous->next == victim);
+            previous->next = victim->next;
+          } else {
+            assert(*dest == victim);
+            *dest = victim->next;
+          }
+
+          victim->next = NULL;
+          addr_policy_free(victim);
+          break;
         }
-
-        victim->next = NULL;
-        addr_policy_free(victim);
-        break;
       }
     }
     if (!tmp) {

src/or/routerlist.c

@@ -2361,10 +2361,11 @@ routerlist_remove_old_cached_routers_with_id(time_t cutoff, int lo, int hi,
     }
   }
 
-  for (i = hi; i >= lo; --i) {
+  i = hi;
+  do {
     if (rmv[i-lo])
       routerlist_remove_old(routerlist, smartlist_get(lst, i), i);
-  }
+  } while (--i >= lo);
   tor_free(must_keep);
   tor_free(rmv);
   tor_free(lifespans);
@@ -4258,17 +4259,22 @@ update_router_descriptor_client_downloads(time_t now)
 
   if (! should_delay) {
     int i, n_per_request;
+    const char *req_plural = "", *rtr_plural = "";
     n_per_request = (n_downloadable+MIN_REQUESTS-1) / MIN_REQUESTS;
     if (n_per_request > MAX_DL_PER_REQUEST)
       n_per_request = MAX_DL_PER_REQUEST;
     if (n_per_request < MIN_DL_PER_REQUEST)
       n_per_request = MIN_DL_PER_REQUEST;
 
+    if (n_downloadable > n_per_request)
+      req_plural = rtr_plural = "s";
+    else if (n_downloadable > 1)
+      rtr_plural = "s";
+
     log_info(LD_DIR,
              "Launching %d request%s for %d router%s, %d at a time",
              (n_downloadable+n_per_request-1)/n_per_request,
-             n_downloadable>n_per_request?"s":"",
-             n_downloadable, n_downloadable>1?"s":"", n_per_request);
+             req_plural, n_downloadable, rtr_plural, n_per_request);
     smartlist_sort_digests(downloadable);
     for (i=0; i < n_downloadable; i += n_per_request) {
       initiate_descriptor_downloads(NULL, downloadable, i, i+n_per_request);