[tor-commits] [tor/master] Move responsibility for config var macros

dgoulet at torproject.org dgoulet at torproject.org
Mon Aug 26 13:43:23 UTC 2019


commit c553750e32d1bf669a3e8308fa44319954a627ca
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Jun 20 15:55:59 2019 -0400

    Move responsibility for config var macros
    
    The testing-only parts now live in a conftesting.h; the shared parts
    of the macros live in confmacros.h
---
 src/app/config/config.c                   | 26 ++--------
 src/app/config/confparse.h                | 39 +-------------
 src/app/config/statefile.c                |  9 +---
 src/feature/dirauth/shared_random_state.c | 12 ++---
 src/lib/conf/.may_include                 |  1 +
 src/lib/conf/confmacros.h                 | 61 ++++++++++++++++++++++
 src/lib/conf/conftesting.h                | 86 +++++++++++++++++++++++++++++++
 src/lib/conf/conftypes.h                  | 37 ++-----------
 src/lib/conf/include.am                   |  4 +-
 src/test/test_confparse.c                 | 12 ++---
 10 files changed, 169 insertions(+), 118 deletions(-)

diff --git a/src/app/config/config.c b/src/app/config/config.c
index 8da1e2acd..c15236b0e 100644
--- a/src/app/config/config.c
+++ b/src/app/config/config.c
@@ -259,28 +259,12 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
  * or_options_t.<b>member</b>"
  */
 #define VAR(varname,conftype,member,initvalue)                          \
-  { { .name = varname,                                                  \
-        .type = CONFIG_TYPE_ ## conftype,                               \
-        .offset = offsetof(or_options_t, member),                       \
-        },                                                              \
-      initvalue CONF_TEST_MEMBERS(or_options_t, conftype, member) }
-
-#ifdef TOR_UNIT_TESTS
-#define DUMMY_TEST_MEMBERS , {.INT=NULL}
-#else
-#define DUMMY_TEST_MEMBERS
-#endif
+  CONFIG_VAR_ETYPE(or_options_t, varname, conftype, member, initvalue)
 
 /* As VAR, but uses a type definition in addition to a type enum. */
 #define VAR_D(varname,conftype,member,initvalue)                        \
-  { { .name = varname,                                                  \
-        .type = CONFIG_TYPE_ ## conftype,                               \
-        .type_def = &conftype ## _type_defn,                            \
-        .offset = offsetof(or_options_t, member),                       \
-        },                                                              \
-      initvalue DUMMY_TEST_MEMBERS }
-
-/** As VAR, but the option name and member name are the same. */
+  CONFIG_VAR_DEFN(or_options_t, varname, conftype, member, initvalue)
+
 #define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 
@@ -289,9 +273,7 @@ DUMMY_TYPECHECK_INSTANCE(or_options_t);
   VAR_D(#member, type, member, initvalue)
 
 /** An entry for config_vars: "The option <b>varname</b> is obsolete." */
-#define OBSOLETE(varname) \
-  { { .name = varname, .type = CONFIG_TYPE_OBSOLETE, }, NULL   \
-    DUMMY_TEST_MEMBERS }
+#define OBSOLETE(varname) CONFIG_VAR_OBSOLETE(varname)
 
 /**
  * Macro to declare *Port options.  Each one comes in three entries.
diff --git a/src/app/config/confparse.h b/src/app/config/confparse.h
index f89ff3c21..6d63ba3e0 100644
--- a/src/app/config/confparse.h
+++ b/src/app/config/confparse.h
@@ -14,6 +14,7 @@
 #define TOR_CONFPARSE_H
 
 #include "lib/conf/conftypes.h"
+#include "lib/conf/confmacros.h"
 
 /** An abbreviation for a configuration option allowed on the command line. */
 typedef struct config_abbrev_t {
@@ -32,44 +33,6 @@ typedef struct config_deprecation_t {
  * you can abbreviate <b>tok</b>s as <b>tok</b>". */
 #define PLURAL(tok) { #tok, #tok "s", 0, 0 }
 
-/* Macros to define extra members inside config_var_t fields, and at the
- * end of a list of them.
- */
-#ifdef TOR_UNIT_TESTS
-/* This is a somewhat magic type-checking macro for users of confparse.c.
- * It initializes a union member "confparse_dummy_values_t.conftype" with
- * the address of a static member "tp_dummy.member".   This
- * will give a compiler warning unless the member field is of the correct
- * type.
- *
- * (This warning is mandatory, because a type mismatch here violates the type
- * compatibility constraint for simple assignment, and requires a diagnostic,
- * according to the C spec.)
- *
- * For example, suppose you say:
- *     "CONF_CHECK_VAR_TYPE(or_options_t, STRING, Address)".
- * Then this macro will evaluate to:
- *     { .STRING = &or_options_t_dummy.Address }
- * And since confparse_dummy_values_t.STRING has type "char **", that
- * expression will create a warning unless or_options_t.Address also
- * has type "char *".
- */
-#define CONF_CHECK_VAR_TYPE(tp, conftype, member)       \
-  { . conftype = &tp ## _dummy . member }
-#define CONF_TEST_MEMBERS(tp, conftype, member) \
-  , CONF_CHECK_VAR_TYPE(tp, conftype, member)
-#define END_OF_CONFIG_VARS                                      \
-  { { .name = NULL }, NULL, { .INT=NULL } }
-#define DUMMY_TYPECHECK_INSTANCE(tp)            \
-  static tp tp ## _dummy
-#else /* !(defined(TOR_UNIT_TESTS)) */
-#define CONF_TEST_MEMBERS(tp, conftype, member)
-#define END_OF_CONFIG_VARS { { .name = NULL, }, NULL }
-/* Repeatedly declarable incomplete struct to absorb redundant semicolons */
-#define DUMMY_TYPECHECK_INSTANCE(tp)            \
-  struct tor_semicolon_eater
-#endif /* defined(TOR_UNIT_TESTS) */
-
 /** Type of a callback to validate whether a given configuration is
  * well-formed and consistent. See options_trial_assign() for documentation
  * of arguments. */
diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c
index 331592c3a..e0584c62a 100644
--- a/src/app/config/statefile.c
+++ b/src/app/config/statefile.c
@@ -70,14 +70,9 @@ static config_abbrev_t state_abbrevs_[] = {
  * members with CONF_CHECK_VAR_TYPE. */
 DUMMY_TYPECHECK_INSTANCE(or_state_t);
 
-/*XXXX these next two are duplicates or near-duplicates from config.c */
 #define VAR(varname,conftype,member,initvalue)                          \
-  { { .name = varname,                                                  \
-      .type = CONFIG_TYPE_ ## conftype,                                 \
-      .offset = offsetof(or_state_t, member), },                        \
-      initvalue CONF_TEST_MEMBERS(or_state_t, conftype, member) }
-/** As VAR, but the option name and member name are the same. */
-#define V(member,conftype,initvalue)                                    \
+  CONFIG_VAR_ETYPE(or_state_t, varname, conftype, member, initvalue)
+#define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
 
 /** Array of "state" variables saved to the ~/.tor/state file. */
diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c
index da4187b38..3cdb223d2 100644
--- a/src/feature/dirauth/shared_random_state.c
+++ b/src/feature/dirauth/shared_random_state.c
@@ -51,15 +51,11 @@ static const char dstate_cur_srv_key[] = "SharedRandCurrentValue";
  * members with CONF_CHECK_VAR_TYPE. */
 DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t);
 
-/* These next two are duplicates or near-duplicates from config.c */
-#define VAR(varname, conftype, member, initvalue)                       \
-  { { .name = varname,                                                  \
-      .type = CONFIG_TYPE_ ## conftype,                                 \
-      .offset = offsetof(sr_disk_state_t, member), },                   \
-    initvalue CONF_TEST_MEMBERS(sr_disk_state_t, conftype, member) }
-/* As VAR, but the option name and member name are the same. */
-#define V(member, conftype, initvalue) \
+#define VAR(varname,conftype,member,initvalue)                          \
+  CONFIG_VAR_ETYPE(sr_disk_state_t, varname, conftype, member, initvalue)
+#define V(member,conftype,initvalue)            \
   VAR(#member, conftype, member, initvalue)
+
 /* Our persistent state magic number. */
 #define SR_DISK_STATE_MAGIC 0x98AB1254
 
diff --git a/src/lib/conf/.may_include b/src/lib/conf/.may_include
index 4285c3dcb..629e2f897 100644
--- a/src/lib/conf/.may_include
+++ b/src/lib/conf/.may_include
@@ -1,2 +1,3 @@
 orconfig.h
 lib/cc/*.h
+lib/conf/*.h
diff --git a/src/lib/conf/confmacros.h b/src/lib/conf/confmacros.h
new file mode 100644
index 000000000..29040e1f5
--- /dev/null
+++ b/src/lib/conf/confmacros.h
@@ -0,0 +1,61 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file confmacros.h
+ * @brief Macro definitions for declaring configuration variables
+ **/
+
+#ifndef TOR_LIB_CONF_CONFMACROS_H
+#define TOR_LIB_CONF_CONFMACROS_H
+
+#include "orconfig.h"
+#include "lib/conf/conftesting.h"
+
+/**
+ * Used to indicate the end of an array of configuration variables.
+ **/
+#define END_OF_CONFIG_VARS                                      \
+  { { .name = NULL }, NULL DUMMY_CONF_TEST_MEMBERS }
+
+/**
+ * Declare a config_var_t as a member named <b>membername</b> of the structure
+ * <b>structtype</b>, whose user-visible name is <b>varname</b>, whose
+ * type corresponds to the config_type_t member CONFIG_TYPE_<b>vartype</b>,
+ * and whose initial value is <b>intval</b>.
+ *
+ * Most modules that use this macro should wrap it in a local macro that
+ * sets structtype to the local configuration type.
+ **/
+#define CONFIG_VAR_ETYPE(structtype, varname, vartype, membername, initval) \
+  { .member =                                                           \
+    { .name = varname,                                                  \
+      .type = CONFIG_TYPE_ ## vartype,                                  \
+      .offset = offsetof(structtype, membername),                       \
+    },                                                                  \
+    .initvalue = initval                                                \
+    CONF_TEST_MEMBERS(structtype, vartype, membername)                  \
+  }
+
+/**
+ * As CONFIG_VAR_XTYPE, but declares a value using an extension type whose
+ * type definition is <b>vartype</b>_type_defn.
+ **/
+#define CONFIG_VAR_DEFN(structtype, varname, vartype, membername, initval) \
+  { .member =                                                           \
+    { .name = varname,                                                  \
+      .type = CONFIG_TYPE_EXTENDED,                                     \
+      .type_def = &vartype ## _type_defn,                               \
+      .offset = offsetof(structtype, membername),                       \
+    },                                                                  \
+    .initvalue = initval                                                \
+    CONF_TEST_MEMBERS(structtype, vartype, membername)                  \
+  }
+
+#define CONFIG_VAR_OBSOLETE(varname)            \
+  { .member = { .name = varname, .type = CONFIG_TYPE_OBSOLETE } }
+
+#endif /* !defined(TOR_LIB_CONF_CONFMACROS_H) */
diff --git a/src/lib/conf/conftesting.h b/src/lib/conf/conftesting.h
new file mode 100644
index 000000000..f4aca442a
--- /dev/null
+++ b/src/lib/conf/conftesting.h
@@ -0,0 +1,86 @@
+/* Copyright (c) 2001 Matej Pfajfar.
+ * Copyright (c) 2001-2004, Roger Dingledine.
+ * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
+ * Copyright (c) 2007-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * @file conftesting.h
+ * @brief Macro and type declarations for testing
+ **/
+
+#ifndef TOR_LIB_CONF_CONFTESTING_H
+#define TOR_LIB_CONF_CONFTESTING_H
+
+#ifdef TOR_UNIT_TESTS
+/**
+ * Union used when building in test mode typechecking the members of a type
+ * used with confparse.c.  See CONF_CHECK_VAR_TYPE for a description of how
+ * it is used. */
+typedef union {
+  char **STRING;
+  char **FILENAME;
+  int *POSINT; /* yes, this is really an int, and not an unsigned int.  For
+                * historical reasons, many configuration values are restricted
+                * to the range [0,INT_MAX], and stored in signed ints.
+                */
+  uint64_t *UINT64;
+  int *INT;
+  int *INTERVAL;
+  int *MSEC_INTERVAL;
+  uint64_t *MEMUNIT;
+  double *DOUBLE;
+  int *BOOL;
+  int *AUTOBOOL;
+  time_t *ISOTIME;
+  struct smartlist_t **CSV;
+  int *CSV_INTERVAL;
+  struct config_line_t **LINELIST;
+  struct config_line_t **LINELIST_S;
+  struct config_line_t **LINELIST_V;
+  // XXXX this doesn't belong at this level of abstraction.
+  struct routerset_t **ROUTERSET;
+} confparse_dummy_values_t;
+#endif /* defined(TOR_UNIT_TESTS) */
+
+/* Macros to define extra members inside config_var_t fields, and at the
+ * end of a list of them.
+ */
+#ifdef TOR_UNIT_TESTS
+/* This is a somewhat magic type-checking macro for users of confparse.c.
+ * It initializes a union member "confparse_dummy_values_t.conftype" with
+ * the address of a static member "tp_dummy.member".   This
+ * will give a compiler warning unless the member field is of the correct
+ * type.
+ *
+ * (This warning is mandatory, because a type mismatch here violates the type
+ * compatibility constraint for simple assignment, and requires a diagnostic,
+ * according to the C spec.)
+ *
+ * For example, suppose you say:
+ *     "CONF_CHECK_VAR_TYPE(or_options_t, STRING, Address)".
+ * Then this macro will evaluate to:
+ *     { .STRING = &or_options_t_dummy.Address }
+ * And since confparse_dummy_values_t.STRING has type "char **", that
+ * expression will create a warning unless or_options_t.Address also
+ * has type "char *".
+ */
+#define CONF_CHECK_VAR_TYPE(tp, conftype, member)       \
+  { . conftype = &tp ## _dummy . member }
+#define CONF_TEST_MEMBERS(tp, conftype, member) \
+  , CONF_CHECK_VAR_TYPE(tp, conftype, member)
+#define DUMMY_CONF_TEST_MEMBERS , { .INT=NULL }
+#define DUMMY_TYPECHECK_INSTANCE(tp)            \
+  static tp tp ## _dummy
+
+#else /* !(defined(TOR_UNIT_TESTS)) */
+
+#define CONF_TEST_MEMBERS(tp, conftype, member)
+/* Repeatedly declarable incomplete struct to absorb redundant semicolons */
+#define DUMMY_TYPECHECK_INSTANCE(tp)            \
+  struct tor_semicolon_eater
+#define DUMMY_CONF_TEST_MEMBERS
+
+#endif /* defined(TOR_UNIT_TESTS) */
+
+#endif /* !defined(TOR_LIB_CONF_CONFTESTING_H) */
diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h
index 5f13ec3de..72697e8ee 100644
--- a/src/lib/conf/conftypes.h
+++ b/src/lib/conf/conftypes.h
@@ -29,6 +29,9 @@
 #define TOR_SRC_LIB_CONF_CONFTYPES_H
 
 #include "lib/cc/torint.h"
+#ifdef TOR_UNIT_TESTS
+#include "lib/conf/conftesting.h"
+#endif
 
 /** Enumeration of types which option values can take */
 typedef enum config_type_t {
@@ -59,9 +62,6 @@ typedef enum config_type_t {
   CONFIG_TYPE_LINELIST_V,   /**< Catch-all "virtual" option to summarize
                              * context-sensitive config lines when fetching.
                              */
-  // XXXX this doesn't belong at this level of abstraction.
-  CONFIG_TYPE_ROUTERSET,    /**< A list of router names, addrs, and fps,
-                             * parsed into a routerset_t. */
   CONFIG_TYPE_OBSOLETE,     /**< Obsolete (ignored) option. */
   CONFIG_TYPE_EXTENDED,     /**< Extended type; definition will appear in
                              * pointer. */
@@ -105,37 +105,6 @@ typedef struct struct_magic_decl_t {
   int magic_offset;
 } struct_magic_decl_t;
 
-#ifdef TOR_UNIT_TESTS
-/**
- * Union used when building in test mode typechecking the members of a type
- * used with confparse.c.  See CONF_CHECK_VAR_TYPE for a description of how
- * it is used. */
-typedef union {
-  char **STRING;
-  char **FILENAME;
-  int *POSINT; /* yes, this is really an int, and not an unsigned int.  For
-                * historical reasons, many configuration values are restricted
-                * to the range [0,INT_MAX], and stored in signed ints.
-                */
-  uint64_t *UINT64;
-  int *INT;
-  int *INTERVAL;
-  int *MSEC_INTERVAL;
-  uint64_t *MEMUNIT;
-  double *DOUBLE;
-  int *BOOL;
-  int *AUTOBOOL;
-  time_t *ISOTIME;
-  struct smartlist_t **CSV;
-  int *CSV_INTERVAL;
-  struct config_line_t **LINELIST;
-  struct config_line_t **LINELIST_S;
-  struct config_line_t **LINELIST_V;
-  // XXXX this doesn't belong at this level of abstraction.
-  struct routerset_t **ROUTERSET;
-} confparse_dummy_values_t;
-#endif /* defined(TOR_UNIT_TESTS) */
-
 /** A variable allowed in the configuration file or on the command line. */
 typedef struct config_var_t {
   struct_member_t member; /** A struct member corresponding to this
diff --git a/src/lib/conf/include.am b/src/lib/conf/include.am
index 25355697d..cb7126184 100644
--- a/src/lib/conf/include.am
+++ b/src/lib/conf/include.am
@@ -1,4 +1,6 @@
 
 # ADD_C_FILE: INSERT HEADERS HERE.
 noinst_HEADERS +=				\
-	src/lib/conf/conftypes.h
+	src/lib/conf/conftesting.h              \
+	src/lib/conf/conftypes.h                \
+	src/lib/conf/confmacros.h
diff --git a/src/test/test_confparse.c b/src/test/test_confparse.c
index d0c33841f..2f408b5b6 100644
--- a/src/test/test_confparse.c
+++ b/src/test/test_confparse.c
@@ -49,13 +49,9 @@ typedef struct test_struct_t {
 static test_struct_t test_struct_t_dummy;
 
 #define VAR(varname,conftype,member,initvalue)                          \
-  { { .name = varname,                                                  \
-        .type = CONFIG_TYPE_##conftype,                                 \
-        .offset = offsetof(test_struct_t, member), },                   \
-      initvalue CONF_TEST_MEMBERS(test_struct_t, conftype, member) }
-
-#define V(name,conftype,initvalue)                                      \
-  VAR( #name, conftype, name, initvalue )
+  CONFIG_VAR_ETYPE(test_struct_t, varname, conftype, member, initvalue)
+#define V(member,conftype,initvalue)            \
+  VAR(#member, conftype, member, initvalue)
 
 #define OBSOLETE(varname)                                               \
   { { .name=varname, .type=CONFIG_TYPE_OBSOLETE }, NULL, {.INT=NULL} }
@@ -83,7 +79,7 @@ static config_var_t test_vars[] = {
   OBSOLETE("obsolete"),
   {
    { .name = "routerset",
-     .type = CONFIG_TYPE_ROUTERSET,
+     .type = CONFIG_TYPE_EXTENDED,
      .type_def = &ROUTERSET_type_defn,
      .offset = offsetof(test_struct_t, routerset),
    },





More information about the tor-commits mailing list