Ticket30935 merged #1254
Ticket30935 merged #1254
Conversation
The testing-only parts now live in a conftesting.h; the shared parts of the macros live in confmacros.h
Additionally, adjust the macros so that we can add new members like this more easily.
"unsettable" is a property of types. LINELIST_V and OBSOLETE are unsettable, meaning that they cannot be set by name. "contained" is a property of types. I'm hoping to find a better name here. LINELIST_S is "contained" because it always appears within a LINELIST_V, and as such doesn't need to be dumped ore copied independently. "cumulative" is a property of types. Cumulative types can appear more than once in a torrc without causing a warning, because they add to each other rather than replacing each other. "obsolete" is a property of variables. "marking fragile" is now a command that struct members can accept. With these changes, confparse and config no longer ever need to mention CONFIG_TYPE_XYZ values by name.
Previously, these were magical things that we detected by checking whether a variable's name was prefixed with two or three underscores.
Previously, when TestingTorNetwork was set, we would manually adjust the initvalue members of a bunch of other config_var_t, and then re-run the early parts or parsing the options. Now we treat the initvalue fields as immutable, but instead assign to them in options_init(), as early as possible. Rather than re-running the early parts of options, we just re-call the options_init_from_string() function. This patch de-kludges some of our code pretty handily. I think it could later handle authorities and fallbacks, but for now I think we should leave those alone.
Now that we have a reasonable implementation for overriding the default options for TestingTorNetwork, we don't need to modify config_var_t structs any more. And therefore, we can have constant format options, like reasonable people.
This C file will eventually belong in lib/confmgt, so it needs to have only low-level dependencies. Now that it no longers needs routerset.c, we can adjust its includes accordingly. I'm not moving the file yet, since it would make fixup commits on earlier branches here really hard to do.
It's weird having obsolete / no dump / invisible options, and then flags that do similar things a higher level of abstraction:
contained = no dump, no copy, no check
not settable = no dump?
And then we have obsolete at this level, which means contained.
I feel like there is a better design that has orthogonal flags, at least at the lowest level. And then the higher levels can use combinations of those flags for contained, settable, obsolete, etc.
| @@ -642,7 +642,7 @@ config_dump(const config_format_t *fmt, const void *default_options, | |||
| continue; | |||
| } | |||
| /* Don't save 'hidden' control variables. */ | |||
This comment is also confusing, we need to distinguish between "invisible" (no read and no dump) and "hidden" (no dump).
Agreed. Must solve now, or should solve before we call this work finished?
Should solve before we call #29211 finished.
| #define CVFLAG_NODUMP (1u<<1) | ||
| /** | ||
| * Flag to indicate that an option is "invisible". An invisible option | ||
| * is always undumpable, and we don't tell the controller about it. |
Why not keep these options orthogonal, and say "no dump" and "no read"?
Agreed. Blocker for this PR, or blocker for calling Tor#29211 done?
Should solve before we call #29211 finished.
| @@ -110,6 +110,16 @@ typedef struct struct_magic_decl_t { | |||
| * fetch this option should produce a warning. | |||
It's weird having obsolete / no dump / invisible options, and then flags that do similar things a higher level of abstraction:
- contained = no dump, no copy, no check
- not settable = no dump?
And then we have obsolete at this level, which means contained.
I feel like there is a better design that has orthogonal flags, at least at the lowest level. And then the higher levels can use combinations of those flags for contained, settable, obsolete, etc.
Agreed. Blocker for this PR, or blocker for calling Tor#29211 done?
Must solve before we call #29211 finished, otherwise this code will be very hard to modify in future.
Pull Request Test Coverage Report for Build 5936
|
No description provided.
The text was updated successfully, but these errors were encountered: