Document inconsistent usage of config_var_is_listable()

See also ticket 31654.

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 55e82934ecf5cb3186d3c5556ea1488445005fb1..72a1190b5d5ff4dff8d1a1c10157368bf593852c 100644 (file)
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -2666,6 +2666,9 @@ list_torrc_options(void)
 {
   smartlist_t *vars = config_mgr_list_vars(get_options_mgr());
   SMARTLIST_FOREACH_BEGIN(vars, const config_var_t *, var) {
+    /* Possibly this should check listable, rather than (or in addition to)
+     * settable. See ticket 31654.
+     */
     if (! config_var_is_settable(var)) {
       /* This variable cannot be set, or cannot be set by this name. */
       continue;
@@ -2680,6 +2683,8 @@ static void
 list_deprecated_options(void)
 {
   smartlist_t *deps = config_mgr_list_deprecated_vars(get_options_mgr());
+  /* Possibly this should check whether the variables are listable,
+   * but currently it does not.  See ticket 31654. */
   SMARTLIST_FOREACH(deps, const char *, name,
                     printf("%s\n", name));
   smartlist_free(deps);
@@ -8142,6 +8147,8 @@ getinfo_helper_config(control_connection_t *conn,
   } else if (!strcmp(question, "config/defaults")) {
     smartlist_t *sl = smartlist_new();
     int dirauth_lines_seen = 0, fallback_lines_seen = 0;
+    /* Possibly this should check whether the variables are listable,
+     * but currently it does not.  See ticket 31654. */
     smartlist_t *vars = config_mgr_list_vars(get_options_mgr());
     SMARTLIST_FOREACH_BEGIN(vars, const config_var_t *, var) {
       if (var->initvalue != NULL) {

diff --git a/src/app/config/confparse.c b/src/app/config/confparse.c
index 930484620b2a1d5df82708a1408e46dd396a59cf..9f7fdc5c35243b99850f614dc45e4ddb53cb1cc3 100644 (file)
--- a/src/app/config/confparse.c
+++ b/src/app/config/confparse.c
@@ -592,6 +592,12 @@ config_var_needs_copy(const config_var_t *var)
 /**
  * Return true iff variable <b>var</b> should appear on list of variable
  * names given to the controller or the CLI.
+ *
+ * (Note that this option is imperfectly obeyed. The
+ * --list-torrc-options command looks at the "settable" flag, whereas
+ * "GETINFO config/defaults" and "list_deprecated_*()" do not filter
+ * their results. It would be good for consistency to try to converge
+ * these behaviors in the future.)
  **/
 bool
 config_var_is_listable(const config_var_t *var)