Change the type of "download schedule" from smartlist to int.

This is done as follows:
  * Only one function (find_dl_schedule()) actually returned a
    smartlist. Now it returns an int.

  * The CSV_INTERVAL type has been altered to ignore everything
    after the first comma, and to store the value before the first
    comma in an int.

diff --git a/src/or/confparse.c b/src/or/confparse.c
index 64ed9ee6bb0ea9d66e305f97737761f596d948fd..6bab7909456607ed0e2c55efcc19a65d87379298 100644 (file)
--- a/src/or/confparse.c
+++ b/src/or/confparse.c
@@ -162,8 +162,6 @@ config_assign_value(const config_format_t *fmt, void *options,
   int i, ok;
   const config_var_t *var;
   void *lvalue;
-  int *csv_int;
-  smartlist_t *csv_str;
 
   CONFIG_CHECK(fmt, options);
 
@@ -195,6 +193,30 @@ config_assign_value(const config_format_t *fmt, void *options,
     *(int *)lvalue = i;
     break;
 
+  case CONFIG_TYPE_CSV_INTERVAL: {
+    /* We used to have entire smartlists here.  But now that all of our
+     * download schedules use exponential backoff, only the first part
+     * matters. */
+    const char *comma = strchr(c->value, ',');
+    const char *val = c->value;
+    char *tmp = NULL;
+    if (comma) {
+      tmp = tor_strndup(c->value, comma - c->value);
+      val = tmp;
+    }
+
+    i = config_parse_interval(val, &ok);
+    if (!ok) {
+      tor_asprintf(msg,
+          "Interval '%s %s' is malformed or out of bounds.",
+          c->key, c->value);
+      return -1;
+    }
+    *(int *)lvalue = i;
+    tor_free(tmp);
+    break;
+  }
+
   case CONFIG_TYPE_INTERVAL: {
     i = config_parse_interval(c->value, &ok);
     if (!ok) {
@@ -298,36 +320,6 @@ config_assign_value(const config_format_t *fmt, void *options,
                            SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
     break;
 
-  case CONFIG_TYPE_CSV_INTERVAL:
-    if (*(smartlist_t**)lvalue) {
-      SMARTLIST_FOREACH(*(smartlist_t**)lvalue, int *, cp, tor_free(cp));
-      smartlist_clear(*(smartlist_t**)lvalue);
-    } else {
-      *(smartlist_t**)lvalue = smartlist_new();
-    }
-    csv_str = smartlist_new();
-    smartlist_split_string(csv_str, c->value, ",",
-                           SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
-    SMARTLIST_FOREACH_BEGIN(csv_str, char *, str)
-      {
-        i = config_parse_interval(str, &ok);
-        if (!ok) {
-          tor_asprintf(msg,
-              "Interval in '%s %s' is malformed or out of bounds.",
-              c->key, c->value);
-          SMARTLIST_FOREACH(csv_str, char *, cp, tor_free(cp));
-          smartlist_free(csv_str);
-          return -1;
-        }
-        csv_int = tor_malloc_zero(sizeof(int));
-        *csv_int = i;
-        smartlist_add(*(smartlist_t**)lvalue, csv_int);
-      }
-    SMARTLIST_FOREACH_END(str);
-    SMARTLIST_FOREACH(csv_str, char *, cp, tor_free(cp));
-    smartlist_free(csv_str);
-    break;
-
   case CONFIG_TYPE_LINELIST:
   case CONFIG_TYPE_LINELIST_S:
     {
@@ -528,7 +520,6 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
   const config_var_t *var;
   const void *value;
   config_line_t *result;
-  smartlist_t *csv_str;
   tor_assert(options && key);
 
   CONFIG_CHECK(fmt, options);
@@ -571,6 +562,7 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
         break;
       }
       /* fall through */
+    case CONFIG_TYPE_CSV_INTERVAL:
     case CONFIG_TYPE_INTERVAL:
     case CONFIG_TYPE_MSEC_INTERVAL:
     case CONFIG_TYPE_UINT:
@@ -611,20 +603,6 @@ config_get_assigned_option(const config_format_t *fmt, const void *options,
       else
         result->value = tor_strdup("");
       break;
-    case CONFIG_TYPE_CSV_INTERVAL:
-      if (*(smartlist_t**)value) {
-        csv_str = smartlist_new();
-        SMARTLIST_FOREACH_BEGIN(*(smartlist_t**)value, int *, i)
-          {
-            smartlist_add_asprintf(csv_str, "%d", *i);
-          }
-        SMARTLIST_FOREACH_END(i);
-        result->value = smartlist_join_strings(csv_str, ",", 0, NULL);
-        SMARTLIST_FOREACH(csv_str, char *, cp, tor_free(cp));
-        smartlist_free(csv_str);
-      } else
-        result->value = tor_strdup("");
-      break;
     case CONFIG_TYPE_OBSOLETE:
       log_fn(LOG_INFO, LD_CONFIG,
              "You asked me for the value of an obsolete config option '%s'.",
@@ -789,6 +767,7 @@ config_clear(const config_format_t *fmt, void *options,
     case CONFIG_TYPE_ISOTIME:
       *(time_t*)lvalue = 0;
       break;
+    case CONFIG_TYPE_CSV_INTERVAL:
     case CONFIG_TYPE_INTERVAL:
     case CONFIG_TYPE_MSEC_INTERVAL:
     case CONFIG_TYPE_UINT:
@@ -816,13 +795,6 @@ config_clear(const config_format_t *fmt, void *options,
         *(smartlist_t **)lvalue = NULL;
       }
       break;
-    case CONFIG_TYPE_CSV_INTERVAL:
-      if (*(smartlist_t**)lvalue) {
-        SMARTLIST_FOREACH(*(smartlist_t **)lvalue, int *, cp, tor_free(cp));
-        smartlist_free(*(smartlist_t **)lvalue);
-        *(smartlist_t **)lvalue = NULL;
-      }
-      break;
     case CONFIG_TYPE_LINELIST:
     case CONFIG_TYPE_LINELIST_S:
       config_free_lines(*(config_line_t **)lvalue);

diff --git a/src/or/confparse.h b/src/or/confparse.h
index f1f20303433136031851ecde7026120a1df5bcd7..64ea65d1b9d46412a1d24b7ce9335b25473c77c3 100644 (file)
--- a/src/or/confparse.h
+++ b/src/or/confparse.h
@@ -62,7 +62,7 @@ typedef union {
   int *AUTOBOOL;
   time_t *ISOTIME;
   smartlist_t **CSV;
-  smartlist_t **CSV_INTERVAL;
+  int *CSV_INTERVAL;
   config_line_t **LINELIST;
   config_line_t **LINELIST_S;
   config_line_t **LINELIST_V;

diff --git a/src/or/directory.c b/src/or/directory.c
index 3e4d978eed60e91d7c9d9ffaa51fabc7796ddeba..7b7dc5b3558da8c662a87c1e8f40ea5bd0df3057 100644 (file)
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -5303,7 +5303,7 @@ connection_dir_finished_connecting(dir_connection_t *conn)
  * Then return a list of int pointers defining download delays in seconds.
  * Helper function for download_status_increment_failure(),
  * download_status_reset(), and download_status_increment_attempt(). */
-STATIC const smartlist_t *
+STATIC int
 find_dl_schedule(const download_status_t *dls, const or_options_t *options)
 {
   switch (dls->schedule) {
@@ -5359,25 +5359,19 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options)
   }
 
   /* Impossible, but gcc will fail with -Werror without a `return`. */
-  return NULL;
+  return 0;
 }
 
 /** Decide which minimum delay step we want to use based on
  * descriptor type in <b>dls</b> and <b>options</b>.
  * Helper function for download_status_schedule_get_delay(). */
 STATIC int
-find_dl_min_delay(download_status_t *dls, const or_options_t *options)
+find_dl_min_delay(const download_status_t *dls, const or_options_t *options)
 {
   tor_assert(dls);
   tor_assert(options);
 
-  /*
-   * For now, just use the existing schedule config stuff and pick the
-   * first/last entries off to get min/max delay for backoff purposes
-   */
-  const smartlist_t *schedule = find_dl_schedule(dls, options);
-  tor_assert(schedule != NULL && smartlist_len(schedule) >= 2);
-  return *(int *)(smartlist_get(schedule, 0));
+  return find_dl_schedule(dls, options);
 }
 
 /** As next_random_exponential_delay() below, but does not compute a random
@@ -5634,10 +5628,9 @@ download_status_increment_attempt(download_status_t *dls, const char *item,
 static time_t
 download_status_get_initial_delay_from_now(const download_status_t *dls)
 {
-  const smartlist_t *schedule = find_dl_schedule(dls, get_options());
   /* We use constant initial delays, even in exponential backoff
    * schedules. */
-  return time(NULL) + *(int *)smartlist_get(schedule, 0);
+  return time(NULL) + find_dl_min_delay(dls, get_options());
 }
 
 /** Reset <b>dls</b> so that it will be considered downloadable

diff --git a/src/or/directory.h b/src/or/directory.h
index aa4d29a5bba50baaf0a52a0d35b06690aa659fa1..e082bbf7b1a613ef6a00ba320d9e41991cd5b50f 100644 (file)
--- a/src/or/directory.h
+++ b/src/or/directory.h
@@ -259,9 +259,9 @@ STATIC char* authdir_type_to_string(dirinfo_type_t auth);
 STATIC const char * dir_conn_purpose_to_string(int purpose);
 STATIC int should_use_directory_guards(const or_options_t *options);
 STATIC compression_level_t choose_compression_level(ssize_t n_bytes);
-STATIC const smartlist_t *find_dl_schedule(const download_status_t *dls,
-                                           const or_options_t *options);
-STATIC int find_dl_min_delay(download_status_t *dls,
+STATIC int find_dl_schedule(const download_status_t *dls,
+                            const or_options_t *options);
+STATIC int find_dl_min_delay(const download_status_t *dls,
                              const or_options_t *options);
 
 STATIC int next_random_exponential_delay(int delay,

diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 5fac045b2637044fc6dd2f3084f07cf5220e0ad4..8926221f052064a82b768018b23096afcf8121eb 100644 (file)
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -4063,34 +4063,19 @@ test_dir_download_status_increment(void *arg)
     DL_WANT_ANY_DIRSERVER,
     DL_SCHED_INCREMENT_ATTEMPT,
     0, 0 };
-  int no_delay = 0;
-  int delay0 = -1;
-  int delay1 = -1;
-  int delay2 = -1;
-  smartlist_t *schedule = smartlist_new();
-  smartlist_t *schedule_no_initial_delay = smartlist_new();
   or_options_t test_options;
   time_t current_time = time(NULL);
 
-  /* Provide some values for the schedules */
-  delay0 = 10;
-  delay1 = 99;
-  delay2 = 20;
-
-  /* Make the schedules */
-  smartlist_add(schedule, (void *)&delay0);
-  smartlist_add(schedule, (void *)&delay1);
-  smartlist_add(schedule, (void *)&delay2);
-
-  smartlist_add(schedule_no_initial_delay, (void *)&no_delay);
-  smartlist_add(schedule_no_initial_delay, (void *)&delay1);
-  smartlist_add(schedule_no_initial_delay, (void *)&delay2);
+  const int delay0 = 10;
+  const int no_delay = 0;
+  const int schedule = 10;
+  const int schedule_no_initial_delay = 0;
 
   /* Put it in the options */
   mock_options = &test_options;
   reset_options(mock_options, &mock_get_options_calls);
-  mock_options->TestingBridgeBootstrapDownloadSchedule = schedule;
-  mock_options->TestingClientDownloadSchedule = schedule;
+  mock_options->TestingBridgeBootstrapDownloadInitialDelay = schedule;
+  mock_options->TestingClientDownloadInitialDelay = schedule;
 
   MOCK(get_options, mock_get_options);
 
@@ -4098,13 +4083,13 @@ test_dir_download_status_increment(void *arg)
    * whether or not it was reset before being used */
 
   /* regression test for 17750: no initial delay */
-  mock_options->TestingClientDownloadSchedule = schedule_no_initial_delay;
+  mock_options->TestingClientDownloadInitialDelay = schedule_no_initial_delay;
   mock_get_options_calls = 0;
   /* we really want to test that it's equal to time(NULL) + delay0, but that's
    * an unrealiable test, because time(NULL) might change. */
 
   /* regression test for 17750: exponential, no initial delay */
-  mock_options->TestingClientDownloadSchedule = schedule_no_initial_delay;
+  mock_options->TestingClientDownloadInitialDelay = schedule_no_initial_delay;
   mock_get_options_calls = 0;
   /* we really want to test that it's equal to time(NULL) + delay0, but that's
    * an unrealiable test, because time(NULL) might change. */
@@ -4117,7 +4102,7 @@ test_dir_download_status_increment(void *arg)
   tt_int_op(mock_get_options_calls, OP_GE, 1);
 
   /* regression test for 17750: exponential, initial delay */
-  mock_options->TestingClientDownloadSchedule = schedule;
+  mock_options->TestingClientDownloadInitialDelay = schedule;
   mock_get_options_calls = 0;
   /* we really want to test that it's equal to time(NULL) + delay0, but that's
    * an unrealiable test, because time(NULL) might change. */
@@ -4130,9 +4115,6 @@ test_dir_download_status_increment(void *arg)
   tt_int_op(mock_get_options_calls, OP_GE, 1);
 
  done:
-  /* the pointers in schedule are allocated on the stack */
-  smartlist_free(schedule);
-  smartlist_free(schedule_no_initial_delay);
   UNMOCK(get_options);
   mock_options = NULL;
   mock_get_options_calls = 0;
@@ -5483,44 +5465,45 @@ test_dir_find_dl_schedule(void* data)
        mock_num_bridges_usable);
 
   download_status_t dls;
-  smartlist_t server, client, server_cons, client_cons;
-  smartlist_t client_boot_auth_only_cons, client_boot_auth_cons;
-  smartlist_t client_boot_fallback_cons, bridge, bridge_bootstrap;
+
+  const int server=10, client=20, server_cons=30, client_cons=40;
+  const int client_boot_auth_only_cons=50, client_boot_auth_cons=60;
+  const int client_boot_fallback_cons=70, bridge=80, bridge_bootstrap=90;
 
   mock_options = tor_malloc(sizeof(or_options_t));
   reset_options(mock_options, &mock_get_options_calls);
   MOCK(get_options, mock_get_options);
 
-  mock_options->TestingServerDownloadSchedule = &server;
-  mock_options->TestingClientDownloadSchedule = &client;
-  mock_options->TestingServerConsensusDownloadSchedule = &server_cons;
-  mock_options->TestingClientConsensusDownloadSchedule = &client_cons;
-  mock_options->ClientBootstrapConsensusAuthorityOnlyDownloadSchedule =
-    &client_boot_auth_only_cons;
-  mock_options->ClientBootstrapConsensusAuthorityDownloadSchedule =
-    &client_boot_auth_cons;
-  mock_options->ClientBootstrapConsensusFallbackDownloadSchedule =
-    &client_boot_fallback_cons;
-  mock_options->TestingBridgeDownloadSchedule = &bridge;
-  mock_options->TestingBridgeBootstrapDownloadSchedule = &bridge_bootstrap;
+  mock_options->TestingServerDownloadInitialDelay = server;
+  mock_options->TestingClientDownloadInitialDelay = client;
+  mock_options->TestingServerConsensusDownloadInitialDelay = server_cons;
+  mock_options->TestingClientConsensusDownloadInitialDelay = client_cons;
+  mock_options->ClientBootstrapConsensusAuthorityOnlyDownloadInitialDelay =
+    client_boot_auth_only_cons;
+  mock_options->ClientBootstrapConsensusAuthorityDownloadInitialDelay =
+    client_boot_auth_cons;
+  mock_options->ClientBootstrapConsensusFallbackDownloadInitialDelay =
+    client_boot_fallback_cons;
+  mock_options->TestingBridgeDownloadInitialDelay = bridge;
+  mock_options->TestingBridgeBootstrapDownloadInitialDelay = bridge_bootstrap;
 
   dls.schedule = DL_SCHED_GENERIC;
   /* client */
   mock_options->ClientOnly = 1;
-  tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &client);
+  tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, client);
   mock_options->ClientOnly = 0;
 
   /* dir mode */
   mock_options->DirPort_set = 1;
   mock_options->DirCache = 1;
-  tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &server);
+  tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, server);
   mock_options->DirPort_set = 0;
   mock_options->DirCache = 0;
 
   dls.schedule = DL_SCHED_CONSENSUS;
   /* public server mode */
   mock_options->ORPort_set = 1;
-  tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &server_cons);
+  tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, server_cons);
   mock_options->ORPort_set = 0;
 
   /* client and bridge modes */
@@ -5529,30 +5512,30 @@ test_dir_find_dl_schedule(void* data)
       dls.want_authority = 1;
       /* client */
       mock_options->ClientOnly = 1;
-      tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-                &client_boot_auth_cons);
+      tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+                client_boot_auth_cons);
       mock_options->ClientOnly = 0;
 
       /* bridge relay */
       mock_options->ORPort_set = 1;
       mock_options->BridgeRelay = 1;
-      tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-                &client_boot_auth_cons);
+      tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+                client_boot_auth_cons);
       mock_options->ORPort_set = 0;
       mock_options->BridgeRelay = 0;
 
       dls.want_authority = 0;
       /* client */
       mock_options->ClientOnly = 1;
-      tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-                &client_boot_fallback_cons);
+      tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+                client_boot_fallback_cons);
       mock_options->ClientOnly = 0;
 
       /* bridge relay */
       mock_options->ORPort_set = 1;
       mock_options->BridgeRelay = 1;
-      tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-                &client_boot_fallback_cons);
+      tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+                client_boot_fallback_cons);
       mock_options->ORPort_set = 0;
       mock_options->BridgeRelay = 0;
 
@@ -5560,30 +5543,30 @@ test_dir_find_dl_schedule(void* data)
       /* dls.want_authority is ignored */
       /* client */
       mock_options->ClientOnly = 1;
-      tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-                &client_boot_auth_only_cons);
+      tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+                client_boot_auth_only_cons);
       mock_options->ClientOnly = 0;
 
       /* bridge relay */
       mock_options->ORPort_set = 1;
       mock_options->BridgeRelay = 1;
-      tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-                &client_boot_auth_only_cons);
+      tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+                client_boot_auth_only_cons);
       mock_options->ORPort_set = 0;
       mock_options->BridgeRelay = 0;
     }
   } else {
     /* client */
     mock_options->ClientOnly = 1;
-    tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-              &client_cons);
+    tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+              client_cons);
     mock_options->ClientOnly = 0;
 
     /* bridge relay */
     mock_options->ORPort_set = 1;
     mock_options->BridgeRelay = 1;
-    tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ,
-              &client_cons);
+    tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ,
+              client_cons);
     mock_options->ORPort_set = 0;
     mock_options->BridgeRelay = 0;
   }
@@ -5593,9 +5576,9 @@ test_dir_find_dl_schedule(void* data)
   mock_options->ClientOnly = 1;
   mock_options->UseBridges = 1;
   if (num_bridges_usable(0) > 0) {
-    tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge);
+    tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, bridge);
   } else {
-    tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge_bootstrap);
+    tt_int_op(find_dl_schedule(&dls, mock_options), OP_EQ, bridge_bootstrap);
   }
 
  done: