From c553750e32d1bf669a3e8308fa44319954a627ca Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 20 Jun 2019 15:55:59 -0400 Subject: [PATCH] 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 | 24 +------ 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, 168 insertions(+), 117 deletions(-) create mode 100644 src/lib/conf/confmacros.h create mode 100644 src/lib/conf/conftesting.h diff --git a/src/app/config/config.c b/src/app/config/config.c index 8da1e2acdc..c15236b0e9 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.member" */ #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 } + CONFIG_VAR_DEFN(or_options_t, varname, conftype, member, initvalue) -/** As VAR, but the option name and member name are the same. */ #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 varname 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 f89ff3c214..6d63ba3e0e 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 toks as tok". */ #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 331592c3a6..e0584c62af 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 da4187b38a..3cdb223d25 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 4285c3dcb8..629e2f897d 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 0000000000..29040e1f55 --- /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 membername of the structure + * structtype, whose user-visible name is varname, whose + * type corresponds to the config_type_t member CONFIG_TYPE_vartype, + * and whose initial value is intval. + * + * 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 vartype_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 0000000000..f4aca442a2 --- /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 5f13ec3de2..72697e8ee4 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 25355697d2..cb7126184d 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 d0c33841fe..2f408b5b6e 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), },