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

Ticket25610 034 01 #58

Closed
wants to merge 18 commits into from
Closed

Conversation

Labels
None yet
Projects
None yet
3 participants
@dgoulet-tor
Copy link
Contributor

@dgoulet-tor dgoulet-tor commented Apr 25, 2018

No description provided.

dgoulet-tor added 18 commits Apr 25, 2018
Make our build system support a disable dirauth module option. It can only be
disabled explicitly with:

  $ ./configure --disable-module-dirauth

If *not* specified that is enabled, an automake conditional variable is set to
true and a defined value for the C code:

  AM_CONDITIONAL: BUILD_MODULE_DIRAUTH
  AC_DEFINE: HAVE_MODULE_DIRAUTH=1

This introduces the dirauth/ module directory in src/or/ for which .c files
are only compiled if the BUILD_MODULE_DIRAUTH is set.

All the header files are compiled in regardless of the support so we can use
the alternative entry point functions of the dirauth subsystem.

Signed-off-by: David Goulet <dgoulet@torproject.org>
Don't access the AuthoritativeDir options directly. We do this so we can move
authdir_mode() to the dirauth module.

Signed-off-by: David Goulet <dgoulet@torproject.org>
Many functions become static to the C file or exposed to the tests within the
PRIVATE define of dirvote.h.

This commit moves a function to the top. No code behavior change.

Signed-off-by: David Goulet <dgoulet@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>
Remove useless include.

Clearly identify functions that are used by other part of Tor, functions that
are only used by the dirauth subsystem and functions that are exposed for unit
tests.

This will help us in the dirauth modularization effort.

Signed-off-by: David Goulet <dgoulet@torproject.org>
This is a pretty big commit but it only moves these files to src/or/dirauth:

  dircollate.c dirvote.c shared_random.c shared_random_state.c
  dircollate.h dirvote.h shared_random.h shared_random_state.h

Then many files are modified to change the include line for those header files
that have moved into a new directory.

Without using --disable-module-dirauth, everything builds fine. When using the
flag to disable the module, tor doesn't build due to linking errors. This will
be addressed in the next commit(s).

No code behavior change.

Signed-off-by: David Goulet <dgoulet@torproject.org>
Move most of the shared random functions that are needed outside of the
dirauth module.

At this commit, because dirvote.c hasn't been refactor, it doesn't compile
because some SR functions need a dirvote function.

Furthermore, 5 functions haven't been touched yet because they are dirauth
only but are in used in other C files than the dirauth module ones.

No code behavior change. Only moving code around.

Signed-off-by: David Goulet <dgoulet@torproject.org>
Add static inline dirauth public functions used outside of the dirauth module
so they can be seen by the tor code but simply do nothing.

Signed-off-by: David Goulet <dgoulet@torproject.org>
No code behavior change.

Signed-off-by: David Goulet <dgoulet@torproject.org>
In order to follow the public namespace of dirvote.

Signed-off-by: David Goulet <dgoulet@torproject.org>
Renamed to follow the file namespace.

Signed-off-by: David Goulet <dgoulet@torproject.org>
From dirvote.c to networkstatus.c where it makes more sense both in terms of
namespace and subsystem responsability.

This removes one less dependency on the dirauth module.

Signed-off-by: David Goulet <dgoulet@torproject.org>
When parsing a vote in routerparse.c, only dirauth extract the commits from
the vote so move all this code into dirvote.c so we can make it specific to
the dirauth module.

If the dirauth module is disabled, the commit parsing does nothing.

Signed-off-by: David Goulet <dgoulet@torproject.org>
In order to make sr_commit_free() only used by the dirauth module, this
commits moves the commits free from a vote object into the dirvote.c file
which is now only for the module.

The function does nothing if the module is disabled.

Signed-off-by: David Goulet <dgoulet@torproject.org>
In order to further isolate the dirauth code into its module, this moves the
handling of the directory request GET /tor/status-vote/* into the module.

Signed-off-by: David Goulet <dgoulet@torproject.org>
Both functions are used for directory request but they can only be used if the
running tor instance is a directory authority.

For this reason, make those symbols visible but hard assert() if they are
called when the module is disabled. This would mean we failed to safeguard the
entry point into the module.

Signed-off-by: David Goulet <dgoulet@torproject.org>
This code is only for dirauth so this commit moves it into the module in
dirvote.c.

No code behavior change.

Signed-off-by: David Goulet <dgoulet@torproject.org>
@coveralls
Copy link

@coveralls coveralls commented Apr 25, 2018

Pull Request Test Coverage Report for Build 579

  • 206 of 501 (41.12%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 58.877%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/or/dirserv.c 2 8 25.0%
src/or/dirvote_common.c 65 77 84.42%
src/or/shared_random_common.c 58 80 72.5%
src/or/dirauth/dirvote.c 65 320 20.31%
Totals Coverage Status
Change from base Build 578: 0.01%
Covered Lines: 40462
Relevant Lines: 68723

💛 - Coveralls

{
(void) options;
(void) now;
return;
Copy link
Member

@ahf ahf Apr 25, 2018

Let's kill these before landing. Only return when there is a value to return.

(void) status_out;
/* If the dirauth module is disabled, this should NEVER be called else we
* failed to safeguard the dirauth module. */
tor_assert(0);
Copy link
Member

@ahf ahf Apr 25, 2018

Good idea, but maybe these should be BUG()?

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Apr 26, 2018

Well there is a backtrace with a tor_assert() but the main point for me was also to stop the whole thing in that case. It should really NEVER happen.

src/or/statefile.h \
src/or/status.h \
src/or/torcert.h \
src/or/tor_api_internal.h

# We add the headers of the modules even though they are disabled so we can
# properly compiled the entry points replacedment.
Copy link
Member

@ahf ahf Apr 25, 2018

I think replacedment is a typo here. Maybe 'stub functions' are better?

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Apr 26, 2018

Yes!

@@ -79,7 +79,14 @@ src_test_AM_CPPFLAGS = -DSHARE_DATADIR="\"$(datadir)\"" \
# This seems to matter nowhere but on Windows, but I assure you that it
# matters a lot there, and is quite hard to debug if you forget to do it.

DIRAUTH_MOD = \
src/or/dirauth/dircollate.c \
Copy link
Member

@ahf ahf Apr 25, 2018

Indentation seems off here? Should be \t.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Apr 26, 2018

Yeah that commit doesn't work and will disappear. Sorry about this, it got forgotten :(

src_test_test_SOURCES = \
$DIRAUTH_MOD \
Copy link
Member

@ahf ahf Apr 25, 2018

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment