Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dirauth config #1607

Closed
wants to merge 10 commits into from
Closed

Dirauth config #1607

wants to merge 10 commits into from

Conversation

Labels
None yet
Projects
None yet
3 participants
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Dec 16, 2019

No description provided.

nmathewson added 6 commits Dec 15, 2019
This change allows other modules to include confdecl.h without
having first to include integer types they might not even use.
These modules are only built when the selected modules are disabled.
The provide stub implementations of the subsystem blocks.  Later,
other stub implementations could move here.

Having real subsystem blocks here will let us handle disabled
configuration options better.
Like "obsolete" variables, these variables produce a warning when
you try to set them, but the warning says that the relevant module
doesn't have support.

The confdecl macros now have a CONF_CONTEXT that you can define to
make all the modules in a given table disabled.
When a subsystem is disabled, there will be no corresponding object
allocated, and no magic numbers on it.
I've chosen the "AuthDirMaxServersPerAddr" option here for
simplicity, since it is used literally nowhere else besides the dirauth
module.  Once we have all the infrastructure in place for this, we
can move more options into this structure.
@coveralls
Copy link

@coveralls coveralls commented Dec 16, 2019

Pull Request Test Coverage Report for Build 7603

  • 14 of 20 (70.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 63.032%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/dirauth/dirvote.c 0 1 0.0%
src/lib/confmgt/confmgt.c 1 2 50.0%
src/lib/confmgt/structvar.c 7 8 87.5%
src/feature/dirauth/dirauth_sys.c 6 9 66.67%
Totals Coverage Status
Change from base Build 7551: 0.03%
Covered Lines: 49579
Relevant Lines: 78657

💛 - Coveralls

@@ -1 +1,2 @@
*.h
*.inc
Copy link
Contributor

@teor2345 teor2345 Dec 18, 2019

I don't think it needs to include any *.inc outside of feature/dirauth/ ?

Suggested change
*.inc
feature/dirauth/*.inc


static const config_format_t dirauth_options_stub_fmt = {
.vars = dirauth_options_t_vars,
};

const struct subsys_fns_t sys_dirauth = {
.name = "dirauth",
.supported = false,
.level = 70,
Copy link
Contributor

@teor2345 teor2345 Dec 18, 2019

If we duplicate the level in *_sys and *_stub, they will get out of sync.

Maybe supported modules shouldn't need a level?
Or maybe they should have an otherwise-illegal level number, using a named #define for readability?

const struct subsys_fns_t sys_relay = {
.name = "relay",
.supported = false,
.level = 50,
Copy link
Contributor

@teor2345 teor2345 Dec 18, 2019

Similar to dirauth_stub, the level should not be duplicated.

nmathewson added 2 commits Dec 18, 2019
This way, we can't get out of sync between the two declarations.
Make the .may_include line for *.inc more specific.
@teor2345 teor2345 closed this Dec 19, 2019
@teor2345 teor2345 reopened this Dec 19, 2019
@teor2345
Copy link
Contributor

@teor2345 teor2345 commented Mar 16, 2020

I think this ticket was squashed and merged?

@teor2345 teor2345 closed this Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment