Commit 6beeb100 authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

Merge branch 'typecheck4'

parents ce4ac7aa 8f0dffe3

changes/ticket23643

0 → 100644
+6 −0
Original line number Diff line number Diff line 
  o Minor features (compilation, testing):

    - Tor builds should now fail if there are any mismatches between the C

      type representing a configuration variable and the C type the

      data-driven parser uses to store a value there.  Previously, we needed

      to check these by hand, which sometimes led to mistakes. Closes ticket

      23643.

src/or/circuitstats.c

+4 −4
Original line number Diff line number Diff line
@@ -910,7 +910,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, 
  int tot_values = 0;

  uint32_t loaded_cnt = 0, N = 0;

  config_line_t *line;

  unsigned int i;

  int i;

  build_time_t *loaded_times;

  int err = 0;

  circuit_build_times_init(cbt);
@@ -960,8 +960,8 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, 
        break;

      }



      if (loaded_cnt+count+state->CircuitBuildAbandonedCount

            > state->TotalBuildTimes) {

      if (loaded_cnt+count+ (unsigned)state->CircuitBuildAbandonedCount

          > (unsigned) state->TotalBuildTimes) {

        log_warn(LD_CIRC,

                 "Too many build times in state file. "

                 "Stopping short before %d",
@@ -986,7 +986,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, 
    loaded_times[loaded_cnt++] = CBT_BUILD_ABANDONED;

  }



  if (loaded_cnt != state->TotalBuildTimes) {

  if (loaded_cnt != (unsigned)state->TotalBuildTimes) {

    log_warn(LD_CIRC,

            "Corrupt state file? Build times count mismatch. "

            "Read %d times, but file says %d", loaded_cnt,

src/or/config.c

+11 −3
Original line number Diff line number Diff line
@@ -174,18 +174,26 @@ static config_abbrev_t option_abbrevs_[] = { 
  { NULL, NULL, 0, 0},

};



/** dummy instance of or_options_t, used for type-checking its

 * members with CONF_CHECK_VAR_TYPE. */

DUMMY_TYPECHECK_INSTANCE(or_options_t);



/** An entry for config_vars: "The option <b>name</b> has type

 * CONFIG_TYPE_<b>conftype</b>, and corresponds to

 * or_options_t.<b>member</b>"

 */

#define VAR(name,conftype,member,initvalue)                             \

  { name, CONFIG_TYPE_ ## conftype, offsetof(or_options_t, member),     \

      initvalue }

      initvalue CONF_TEST_MEMBERS(or_options_t, conftype, member) }

/** As VAR, but the option name and member name are the same. */

#define V(member,conftype,initvalue)                                    \

  VAR(#member, conftype, member, initvalue)

/** An entry for config_vars: "The option <b>name</b> is obsolete." */

#ifdef TOR_UNIT_TESTS

#define OBSOLETE(name) { name, CONFIG_TYPE_OBSOLETE, 0, NULL, {.INT=NULL} }

#else

#define OBSOLETE(name) { name, CONFIG_TYPE_OBSOLETE, 0, NULL }

#endif



/**

 * Macro to declare *Port options.  Each one comes in three entries.
@@ -621,7 +629,7 @@ static config_var_t option_vars_[] = { 
  V(TestingDirAuthVoteHSDirIsStrict,  BOOL,     "0"),

  VAR("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_, "0"),



  { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL }

  END_OF_CONFIG_VARS

};



/** Override default values with these if the user sets the TestingTorNetwork
@@ -676,7 +684,7 @@ static const config_var_t testing_tor_network_defaults[] = { 
  VAR("___UsingTestNetworkDefaults", BOOL, UsingTestNetworkDefaults_, "1"),

  V(RendPostPeriod,              INTERVAL, "2 minutes"),



  { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL }

  END_OF_CONFIG_VARS

};



#undef VAR

src/or/confparse.h

+72 −0
Original line number Diff line number Diff line
@@ -40,6 +40,36 @@ typedef enum config_type_t { 
  CONFIG_TYPE_OBSOLETE,     /**< Obsolete (ignored) option. */

} config_type_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 *UINT; /* yes, really: Even though the confparse type is called

              * "UINT", it still uses the C int type -- it just enforces that

              * the values are in range [0,INT_MAX].

              */

  int *INT;

  int *PORT;

  int *INTERVAL;

  int *MSEC_INTERVAL;

  uint64_t *MEMUNIT;

  double *DOUBLE;

  int *BOOL;

  int *AUTOBOOL;

  time_t *ISOTIME;

  smartlist_t **CSV;

  smartlist_t **CSV_INTERVAL;

  config_line_t **LINELIST;

  config_line_t **LINELIST_S;

  config_line_t **LINELIST_V;

  routerset_t **ROUTERSET;

} confparse_dummy_values_t;

#endif



/** An abbreviation for a configuration option allowed on the command line. */

typedef struct config_abbrev_t {

  const char *abbreviated;
@@ -64,8 +94,50 @@ typedef struct config_var_t { 
                       * value. */

  off_t var_offset; /**< Offset of the corresponding member of or_options_t. */

  const char *initvalue; /**< String (or null) describing initial value. */



#ifdef TOR_UNIT_TESTS

  /** Used for compiler-magic to typecheck the corresponding field in the

   * corresponding struct. Only used in unit test mode, at compile-time. */

  confparse_dummy_values_t var_ptr_dummy;

#endif

} config_var_t;



/* 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                                      \

  { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL, { .INT=NULL } }

#define DUMMY_TYPECHECK_INSTANCE(tp)            \

  static tp tp ## _dummy

#else

#define CONF_TEST_MEMBERS(tp, conftype, member)

#define END_OF_CONFIG_VARS { NULL, CONFIG_TYPE_OBSOLETE, 0, NULL }

#define DUMMY_TYPECHECK_INSTANCE(tp)

#endif



/** Type of a callback to validate whether a given configuration is

 * well-formed and consistent. See options_trial_assign() for documentation

 * of arguments. */

src/or/or.h

+4 −4
Original line number Diff line number Diff line
@@ -3704,7 +3704,7 @@ typedef struct { 
  config_line_t *SocksPort_lines;

  /** Ports to listen on for transparent pf/netfilter connections. */

  config_line_t *TransPort_lines;

  const char *TransProxyType; /**< What kind of transparent proxy

  char *TransProxyType; /**< What kind of transparent proxy

                         * implementation are we using? */

  /** Parsed value of TransProxyType. */

  enum {
@@ -4686,8 +4686,8 @@ typedef struct { 


  /** Build time histogram */

  config_line_t * BuildtimeHistogram;

  unsigned int TotalBuildTimes;

  unsigned int CircuitBuildAbandonedCount;

  int TotalBuildTimes;

  int CircuitBuildAbandonedCount;



  /** What version of Tor wrote this state file? */

  char *TorVersion;