From f7d654b81e1a65803c22bb53fc1d3a2021d87d50 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 6 Jun 2013 14:56:05 -0400
Subject: [PATCH 1/6] Start work on fancy compiler tricks to expose extra stuff
 to our tests

This is mainly a matter of automake trickery: we build each static
library in two versions now: one with the TOR_UNIT_TESTS macro
defined, and one without.  When TOR_UNIT_TESTS is defined, we can
enable mocking and expose more functions. When it's not defined, we
can lock the binary down more.

The alternatives would be to have alternate build modes: a "testing
configuration" for building the libraries with test support, and a
"production configuration" for building them without.  I don't favor
that approach, since I think it would mean more people runnning
binaries build for testing, or more people not running unit tests.
---
 .gitignore               |  4 ++++
 changes/fancy_testing    |  9 +++++++++
 configure.ac             |  4 ++++
 src/common/include.am    | 31 +++++++++++++++++++++++++++----
 src/common/testsupport.h | 14 ++++++++++++++
 src/or/include.am        | 14 ++++++++++++--
 src/test/include.am      | 14 +++++++++-----
 7 files changed, 79 insertions(+), 11 deletions(-)
 create mode 100644 changes/fancy_testing
 create mode 100644 src/common/testsupport.h

diff --git a/.gitignore b/.gitignore
index e48918d32c..bb3817ba57 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,10 +128,13 @@
 /src/common/Makefile.in
 /src/common/common_sha1.i
 /src/common/libor.a
+/src/common/libor-testing.a
 /src/common/libor.lib
 /src/common/libor-crypto.a
+/src/common/libor-crypto-testing.a
 /src/common/libor-crypto.lib
 /src/common/libor-event.a
+/src/common/libor-event-testing.a
 /src/common/libor-event.lib
 /src/common/libcurve25519_donna.a
 /src/common/libcurve25519_donna.lib
@@ -150,6 +153,7 @@
 /src/or/tor
 /src/or/tor.exe
 /src/or/libtor.a
+/src/or/libtor-testing.a
 /src/or/libtor.lib
 
 # /src/test
diff --git a/changes/fancy_testing b/changes/fancy_testing
new file mode 100644
index 0000000000..4d971943bd
--- /dev/null
+++ b/changes/fancy_testing
@@ -0,0 +1,9 @@
+  o Build features:
+
+    - Tor now builds each source file in two modes: a mode that avoids
+      exposing identifiers needlessly, and another mode that exposes
+      more identifiers for testing. This lets the compiler do better at
+      optimizing the production code, while enabling us to take more
+      radical measures to let the unit tests test things.
+
+
diff --git a/configure.ac b/configure.ac
index 235f19b438..34ed524dd8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -39,6 +39,10 @@ AC_ARG_ENABLE(static-tor,
    AS_HELP_STRING(--enable-static-tor, Create an entirely static Tor binary. Requires --with-openssl-dir and --with-libevent-dir and --with-zlib-dir))
 AC_ARG_ENABLE(curve25519,
    AS_HELP_STRING(--disable-curve25519, Build Tor with no curve25519 elliptic-curve crypto support))
+AC_ARG_ENABLE(unittests,
+   AS_HELP_STRING(--disable-unittests, [Don't build unit tests for Tor. Risky!]))
+
+AM_CONDITIONAL(UNITTESTS_ENABLED, test x$unittests != xno)
 
 if test "$enable_static_tor" = "yes"; then
   enable_static_libevent="yes";
diff --git a/src/common/include.am b/src/common/include.am
index 68275cbcf7..3d8cc8e9eb 100644
--- a/src/common/include.am
+++ b/src/common/include.am
@@ -1,5 +1,15 @@
 
-noinst_LIBRARIES+= src/common/libor.a src/common/libor-crypto.a src/common/libor-event.a
+noinst_LIBRARIES += \
+	src/common/libor.a \
+	src/common/libor-crypto.a \
+	src/common/libor-event.a
+
+if UNITTESTS_ENABLED
+noinst_LIBRARIES += \
+	src/common/libor-testing.a \
+	src/common/libor-crypto-testing.a \
+	src/common/libor-event-testing.a
+endif
 
 EXTRA_DIST+= \
   src/common/common_sha1.i	\
@@ -38,7 +48,7 @@ if CURVE25519_ENABLED
 libcrypto_extra_source=src/common/crypto_curve25519.c
 endif
 
-src_common_libor_a_SOURCES = \
+LIBOR_A_SOURCES = \
   src/common/address.c					\
   src/common/compat.c					\
   src/common/container.c				\
@@ -51,7 +61,7 @@ src_common_libor_a_SOURCES = \
   src/common/util_codedigest.c				\
   $(libor_extra_source)
 
-src_common_libor_crypto_a_SOURCES = \
+LIBOR_CRYPTO_A_SOURCES = \
   src/common/aes.c		\
   src/common/crypto.c		\
   src/common/crypto_format.c	\
@@ -59,7 +69,19 @@ src_common_libor_crypto_a_SOURCES = \
   src/common/tortls.c		\
   $(libcrypto_extra_source)
 
-src_common_libor_event_a_SOURCES = src/common/compat_libevent.c
+LIBOR_EVENT_A_SOURCES = src/common/compat_libevent.c
+
+src_common_libor_a_SOURCES = $(LIBOR_A_SOURCES)
+src_common_libor_crypto_a_SOURCES = $(LIBOR_CRYPTO_A_SOURCES)
+src_common_libor_event_a_SOURCES = $(LIBOR_EVENT_A_SOURCES)
+
+src_common_libor_testing_a_SOURCES = $(LIBOR_A_SOURCES)
+src_common_libor_crypto_testing_a_SOURCES = $(LIBOR_CRYPTO_A_SOURCES)
+src_common_libor_event_testing_a_SOURCES = $(LIBOR_EVENT_A_SOURCES)
+
+src_common_libor_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
+src_common_libor_crypto_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
+src_common_libor_event_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
 
 COMMONHEADERS = \
   src/common/address.h				\
@@ -74,6 +96,7 @@ COMMONHEADERS = \
   src/common/memarea.h				\
   src/common/mempool.h				\
   src/common/procmon.h				\
+  src/common/testsupport.h			\
   src/common/torgzip.h				\
   src/common/torint.h				\
   src/common/torlog.h				\
diff --git a/src/common/testsupport.h b/src/common/testsupport.h
new file mode 100644
index 0000000000..621fd8c833
--- /dev/null
+++ b/src/common/testsupport.h
@@ -0,0 +1,14 @@
+/* Copyright (c) 2013, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#ifndef TOR_TESTSUPPORT_H
+#define TOR_TESTSUPPORT_H
+
+#ifdef TOR_UNIT_TESTS
+#define STATIC_UNLESS_TESTING
+#else
+#define STATIC_UNLESS_TESTING static
+#endif
+
+#endif
+
diff --git a/src/or/include.am b/src/or/include.am
index 65dbeff53e..91b9bfce72 100644
--- a/src/or/include.am
+++ b/src/or/include.am
@@ -1,5 +1,10 @@
 bin_PROGRAMS+= src/or/tor
-noinst_LIBRARIES+= src/or/libtor.a
+noinst_LIBRARIES += \
+	src/or/libtor.a
+if UNITTESTS_ENABLED
+noinst_LIBRARIES += \
+	src/or/libtor-testing.a
+endif
 
 if BUILD_NT_SERVICES
 tor_platform_source=src/or/ntmain.c
@@ -21,7 +26,7 @@ else
 onion_ntor_source=
 endif
 
-src_or_libtor_a_SOURCES = \
+LIBTOR_A_SOURCES = \
 	src/or/addressmap.c				\
 	src/or/buffers.c				\
 	src/or/channel.c				\
@@ -77,6 +82,9 @@ src_or_libtor_a_SOURCES = \
 	$(onion_ntor_source)				\
 	src/or/config_codedigest.c
 
+src_or_libtor_a_SOURCES = $(LIBTOR_A_SOURCES)
+src_or_libtor_testing_a_SOURCES = $(LIBTOR_A_SOURCES)
+
 #libtor_a_LIBADD = ../common/libor.a ../common/libor-crypto.a \
 #	../common/libor-event.a
 
@@ -90,6 +98,8 @@ AM_CPPFLAGS += -DSHARE_DATADIR="\"$(datadir)\"" \
         -DLOCALSTATEDIR="\"$(localstatedir)\"" \
         -DBINDIR="\"$(bindir)\""
 
+src_or_libtor_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
+
 # -L flags need to go in LDFLAGS. -l flags need to go in LDADD.
 # 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.
diff --git a/src/test/include.am b/src/test/include.am
index 112d1a79d8..08eb7fba67 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -1,11 +1,15 @@
 TESTS+= src/test/test
 
-noinst_PROGRAMS+= src/test/test src/test/test-child src/test/bench
+noinst_PROGRAMS+= src/test/bench
+if UNITTESTS_ENABLED
+noinst_PROGRAMS+= src/test/test src/test/test-child
+endif
 
 src_test_AM_CPPFLAGS = -DSHARE_DATADIR="\"$(datadir)\"" \
         -DLOCALSTATEDIR="\"$(localstatedir)\"" \
         -DBINDIR="\"$(bindir)\""	       \
-	-I"$(top_srcdir)/src/or" -I"$(top_srcdir)/src/ext"
+	-I"$(top_srcdir)/src/or" -I"$(top_srcdir)/src/ext" \
+	-DTOR_UNIT_TESTS
 
 # -L flags need to go in LDFLAGS. -l flags need to go in LDADD.
 # This seems to matter nowhere but on Windows, but I assure you that it
@@ -36,9 +40,9 @@ src_test_bench_CPPFLAGS= $(src_test_AM_CPPFLAGS)
 
 src_test_test_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ \
         @TOR_LDFLAGS_libevent@
-src_test_test_LDADD = src/or/libtor.a src/common/libor.a \
-	src/common/libor-crypto.a $(LIBDONNA) \
-	src/common/libor-event.a \
+src_test_test_LDADD = src/or/libtor-testing.a src/common/libor-testing.a \
+	src/common/libor-crypto-testing.a $(LIBDONNA) \
+	src/common/libor-event-testing.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ \
 	@TOR_OPENSSL_LIBS@ @TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@
 

From a3e0a87d951b1323ca542c9b115d5525d0d022c9 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 6 Jun 2013 17:58:28 -0400
Subject: [PATCH 2/6] Completely refactor how FILENAME_PRIVATE works

We previously used FILENAME_PRIVATE identifiers mostly for
identifiers exposed only to the unit tests... but also for
identifiers exposed to the benchmarker, and sometimes for
identifiers exposed to a similar module, and occasionally for no
really good reason at all.

Now, we use FILENAME_PRIVATE identifiers for identifiers shared by
Tor and the unit tests.  They should be defined static when we
aren't building the unit test, and globally visible otherwise. (The
STATIC macro will keep us honest here.)

For identifiers used only by the unit tests and never by Tor at all,
on the other hand, we wrap them in #ifdef TOR_UNIT_TESTS.

This is not the motivating use case for the split test/non-test
build system; it's just a test example to see how it works, and to
take a chance to clean up the code a little.
---
 changes/fancy_testing          |  3 +++
 src/common/crypto.c            |  6 +++---
 src/common/crypto.h            |  7 +++----
 src/common/crypto_curve25519.c |  2 +-
 src/common/crypto_curve25519.h |  5 +++--
 src/common/crypto_format.c     |  1 -
 src/common/log.c               |  4 ++++
 src/common/testsupport.h       |  4 ++--
 src/common/torlog.h            |  6 ------
 src/common/tortls.c            |  3 ---
 src/common/util.c              |  4 ++--
 src/common/util.h              |  5 +++--
 src/or/addressmap.c            |  2 +-
 src/or/addressmap.h            |  6 ++++--
 src/or/buffers.c               |  2 +-
 src/or/buffers.h               |  4 +++-
 src/or/circuitstats.c          | 21 +++++++++++++++++----
 src/or/circuitstats.h          | 27 ++++++++++++++-------------
 src/or/config.c                |  2 --
 src/or/config.h                |  3 ---
 src/or/connection_edge.c       |  4 ++--
 src/or/connection_edge.h       |  6 ++++--
 src/or/control.c               |  4 ++--
 src/or/control.h               |  4 ++--
 src/or/dirserv.c               | 18 +++++++++---------
 src/or/dirserv.h               | 23 +++++++++++++----------
 src/or/dirvote.c               |  4 ++--
 src/or/dirvote.h               |  6 ++++--
 src/or/geoip.c                 |  6 +++---
 src/or/geoip.h                 |  8 +++++---
 src/or/hibernate.c             |  2 ++
 src/or/hibernate.h             |  2 ++
 src/or/main.c                  |  9 ++++-----
 src/or/main.h                  |  4 ----
 src/or/ntmain.c                |  1 -
 src/or/relay.c                 |  2 +-
 src/or/relay.h                 |  5 +++--
 src/or/replaycache.c           |  4 ++--
 src/or/replaycache.h           |  4 ++--
 src/or/router.c                |  2 +-
 src/or/router.h                |  4 +++-
 src/or/routerlist.c            |  4 ++--
 src/or/routerlist.h            |  9 ++++++---
 src/or/transports.c            | 10 +++++-----
 src/or/transports.h            | 10 +++++-----
 src/test/bench.c               |  1 -
 src/test/test.c                |  1 -
 src/test/test_crypto.c         |  3 +--
 src/tools/tor-checkkey.c       |  2 --
 src/tools/tor-gencert.c        |  2 --
 50 files changed, 149 insertions(+), 132 deletions(-)

diff --git a/changes/fancy_testing b/changes/fancy_testing
index 4d971943bd..3e8ccbca48 100644
--- a/changes/fancy_testing
+++ b/changes/fancy_testing
@@ -6,4 +6,7 @@
       optimizing the production code, while enabling us to take more
       radical measures to let the unit tests test things.
 
+    - The production builds no longer include functions used only
+      in the unit tests; all functions exposed from a module for
+      unit-testing only are now static in production builds.
 
diff --git a/src/common/crypto.c b/src/common/crypto.c
index e60172b744..730ce08286 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -1232,8 +1232,8 @@ crypto_pk_get_all_digests(crypto_pk_t *pk, digests_t *digests_out)
 
 /** Copy <b>in</b> to the <b>outlen</b>-byte buffer <b>out</b>, adding spaces
  * every four spaces. */
-/* static */ void
-add_spaces_to_fp(char *out, size_t outlen, const char *in)
+void
+crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in)
 {
   int n = 0;
   char *end = out+outlen;
@@ -1270,7 +1270,7 @@ crypto_pk_get_fingerprint(crypto_pk_t *pk, char *fp_out, int add_space)
   }
   base16_encode(hexdigest,sizeof(hexdigest),digest,DIGEST_LEN);
   if (add_space) {
-    add_spaces_to_fp(fp_out, FINGERPRINT_LEN+1, hexdigest);
+    crypto_add_spaces_to_fp(fp_out, FINGERPRINT_LEN+1, hexdigest);
   } else {
     strncpy(fp_out, hexdigest, HEX_DIGEST_LEN+1);
   }
diff --git a/src/common/crypto.h b/src/common/crypto.h
index 2886306a6a..651d553a4e 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -15,6 +15,7 @@
 
 #include <stdio.h>
 #include "torint.h"
+#include "testsupport.h"
 
 /*
   Macro to create an arbitrary OpenSSL version number as used by
@@ -283,7 +284,6 @@ void secret_to_key(char *key_out, size_t key_out_len, const char *secret,
 /** OpenSSL-based utility functions. */
 void memwipe(void *mem, uint8_t byte, size_t sz);
 
-#ifdef CRYPTO_PRIVATE
 /* Prototypes for private functions only used by tortls.c, crypto.c, and the
  * unit tests. */
 struct rsa_st;
@@ -294,9 +294,8 @@ crypto_pk_t *crypto_new_pk_from_rsa_(struct rsa_st *rsa);
 struct evp_pkey_st *crypto_pk_get_evp_pkey_(crypto_pk_t *env,
                                                 int private);
 struct dh_st *crypto_dh_get_dh_(crypto_dh_t *dh);
-/* Prototypes for private functions only used by crypto.c and test.c*/
-void add_spaces_to_fp(char *out, size_t outlen, const char *in);
-#endif
+
+void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in);
 
 #endif
 
diff --git a/src/common/crypto_curve25519.c b/src/common/crypto_curve25519.c
index 88c723f37c..9e83440e16 100644
--- a/src/common/crypto_curve25519.c
+++ b/src/common/crypto_curve25519.c
@@ -29,7 +29,7 @@ int curve25519_donna(uint8_t *mypublic,
 #endif
 #endif
 
-int
+STATIC int
 curve25519_impl(uint8_t *output, const uint8_t *secret,
                 const uint8_t *basepoint)
 {
diff --git a/src/common/crypto_curve25519.h b/src/common/crypto_curve25519.h
index 652f1883c6..f9d533ba22 100644
--- a/src/common/crypto_curve25519.h
+++ b/src/common/crypto_curve25519.h
@@ -4,6 +4,7 @@
 #ifndef TOR_CRYPTO_CURVE25519_H
 #define TOR_CRYPTO_CURVE25519_H
 
+#include "testsupport.h"
 #include "torint.h"
 
 /** Length of a curve25519 public key when encoded. */
@@ -52,8 +53,8 @@ int curve25519_keypair_read_from_file(curve25519_keypair_t *keypair_out,
                                       const char *fname);
 
 #ifdef CRYPTO_CURVE25519_PRIVATE
-int curve25519_impl(uint8_t *output, const uint8_t *secret,
-                    const uint8_t *basepoint);
+STATIC int curve25519_impl(uint8_t *output, const uint8_t *secret,
+                           const uint8_t *basepoint);
 #endif
 #endif
 
diff --git a/src/common/crypto_format.c b/src/common/crypto_format.c
index 93932f839c..be669c8d2b 100644
--- a/src/common/crypto_format.c
+++ b/src/common/crypto_format.c
@@ -3,7 +3,6 @@
 
 /* Formatting and parsing code for crypto-related data structures. */
 
-#define CRYPTO_CURVE25519_PRIVATE
 #include "orconfig.h"
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
diff --git a/src/common/log.c b/src/common/log.c
index e196a11287..289c08aa01 100644
--- a/src/common/log.c
+++ b/src/common/log.c
@@ -36,6 +36,10 @@
 #include "torlog.h"
 #include "container.h"
 
+/** Given a severity, yields an index into log_severity_list_t.masks to use
+ * for that severity. */
+#define SEVERITY_MASK_IDX(sev) ((sev) - LOG_ERR)
+
 /** @{ */
 /** The string we stick at the end of a log message when it is too long,
  * and its length. */
diff --git a/src/common/testsupport.h b/src/common/testsupport.h
index 621fd8c833..c6777d0cb2 100644
--- a/src/common/testsupport.h
+++ b/src/common/testsupport.h
@@ -5,9 +5,9 @@
 #define TOR_TESTSUPPORT_H
 
 #ifdef TOR_UNIT_TESTS
-#define STATIC_UNLESS_TESTING
+#define STATIC
 #else
-#define STATIC_UNLESS_TESTING static
+#define STATIC static
 #endif
 
 #endif
diff --git a/src/common/torlog.h b/src/common/torlog.h
index 8675d7b6e7..3ea3e22921 100644
--- a/src/common/torlog.h
+++ b/src/common/torlog.h
@@ -114,12 +114,6 @@ typedef struct log_severity_list_t {
   log_domain_mask_t masks[LOG_DEBUG-LOG_ERR+1];
 } log_severity_list_t;
 
-#ifdef LOG_PRIVATE
-/** Given a severity, yields an index into log_severity_list_t.masks to use
- * for that severity. */
-#define SEVERITY_MASK_IDX(sev) ((sev) - LOG_ERR)
-#endif
-
 /** Callback type used for add_callback_log. */
 typedef void (*log_callback)(int severity, uint32_t domain, const char *msg);
 
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 6bd557b8c0..df706b0012 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -48,9 +48,6 @@
 #include "compat_libevent.h"
 #endif
 
-#define CRYPTO_PRIVATE /* to import prototypes from crypto.h */
-#define TORTLS_PRIVATE
-
 #include "crypto.h"
 #include "tortls.h"
 #include "util.h"
diff --git a/src/common/util.c b/src/common/util.c
index 651554ed23..f5bacfa922 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -3402,7 +3402,7 @@ tor_join_win_cmdline(const char *argv[])
  * function; it's designed to be used in code paths where you can't call
  * arbitrary C functions.
  */
-int
+STATIC int
 format_hex_number_for_helper_exit_status(unsigned int x, char *buf,
                                          int max_len)
 {
@@ -3458,7 +3458,7 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf,
  * On success return the number of characters added to hex_errno, not counting
  * the terminating NUL; return -1 on error.
  */
-int
+STATIC int
 format_helper_exit_status(unsigned char child_state, int saved_errno,
                           char *hex_errno)
 {
diff --git a/src/common/util.h b/src/common/util.h
index 5596378bca..ca6ce7c4c8 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -15,6 +15,7 @@
 #include "torint.h"
 #include "compat.h"
 #include "di_ops.h"
+#include "testsupport.h"
 #include <stdio.h>
 #include <stdlib.h>
 #ifdef _WIN32
@@ -520,9 +521,9 @@ int32_t tor_weak_random_range(tor_weak_rng_t *rng, int32_t top);
 #ifdef UTIL_PRIVATE
 /* Prototypes for private functions only used by util.c (and unit tests) */
 
-int format_hex_number_for_helper_exit_status(unsigned int x, char *buf,
+STATIC int format_hex_number_for_helper_exit_status(unsigned int x, char *buf,
                                              int max_len);
-int format_helper_exit_status(unsigned char child_state,
+STATIC int format_helper_exit_status(unsigned char child_state,
                               int saved_errno, char *hex_errno);
 
 /* Space for hex values of child state, a slash, saved_errno (with
diff --git a/src/or/addressmap.c b/src/or/addressmap.c
index 79e4b7c5e2..9bc79bd84b 100644
--- a/src/or/addressmap.c
+++ b/src/or/addressmap.c
@@ -798,7 +798,7 @@ address_is_in_virtual_range(const char *address)
 /** Return a random address conforming to the virtual address configuration
  * in <b>conf</b>.
  */
-/* private */ void
+STATIC void
 get_random_virtual_addr(const virtual_addr_conf_t *conf, tor_addr_t *addr_out)
 {
   uint8_t tmp[4];
diff --git a/src/or/addressmap.h b/src/or/addressmap.h
index 40210ee990..417832b31f 100644
--- a/src/or/addressmap.h
+++ b/src/or/addressmap.h
@@ -7,6 +7,8 @@
 #ifndef TOR_ADDRESSMAP_H
 #define TOR_ADDRESSMAP_H
 
+#include "testsupport.h"
+
 void addressmap_init(void);
 void addressmap_clear_excluded_trackexithosts(const or_options_t *options);
 void addressmap_clear_invalid_automaps(const or_options_t *options);
@@ -52,8 +54,8 @@ typedef struct virtual_addr_conf_t {
   maskbits_t bits;
 } virtual_addr_conf_t;
 
-void get_random_virtual_addr(const virtual_addr_conf_t *conf,
-                             tor_addr_t *addr_out);
+STATIC void get_random_virtual_addr(const virtual_addr_conf_t *conf,
+                                    tor_addr_t *addr_out);
 #endif
 
 #endif
diff --git a/src/or/buffers.c b/src/or/buffers.c
index c4c847ec87..cc5890416f 100644
--- a/src/or/buffers.c
+++ b/src/or/buffers.c
@@ -1294,7 +1294,7 @@ buf_matches_at_pos(const buf_pos_t *pos, const char *s, size_t n)
 
 /** Return the first position in <b>buf</b> at which the <b>n</b>-character
  * string <b>s</b> occurs, or -1 if it does not occur. */
-/*private*/ int
+STATIC int
 buf_find_string_offset(const buf_t *buf, const char *s, size_t n)
 {
   buf_pos_t pos;
diff --git a/src/or/buffers.h b/src/or/buffers.h
index c947f0ba98..910494a874 100644
--- a/src/or/buffers.h
+++ b/src/or/buffers.h
@@ -12,6 +12,8 @@
 #ifndef TOR_BUFFERS_H
 #define TOR_BUFFERS_H
 
+#include "testsupport.h"
+
 buf_t *buf_new(void);
 buf_t *buf_new_with_capacity(size_t size);
 void buf_free(buf_t *buf);
@@ -89,7 +91,7 @@ int generic_buffer_set_to_copy(generic_buffer_t **output,
 void assert_buf_ok(buf_t *buf);
 
 #ifdef BUFFERS_PRIVATE
-int buf_find_string_offset(const buf_t *buf, const char *s, size_t n);
+STATIC int buf_find_string_offset(const buf_t *buf, const char *s, size_t n);
 #endif
 
 #endif
diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c
index 1d7812bf2b..95129f9a43 100644
--- a/src/or/circuitstats.c
+++ b/src/or/circuitstats.c
@@ -29,9 +29,13 @@
 /* XXXX024 Make this static; add accessor functions. */
 circuit_build_times_t circ_times;
 
+#ifdef TOR_UNIT_TESTS
 /** If set, we're running the unit tests: we should avoid clobbering
  * our state file or accessing get_options() or get_or_state() */
 static int unit_tests = 0;
+#else
+#define unit_tests 0
+#endif
 
 /**
  * This function decides if CBT learning should be disabled. It returns
@@ -438,7 +442,7 @@ circuit_build_times_get_initial_timeout(void)
  * Leave estimated parameters, timeout and network liveness intact
  * for future use.
  */
-void
+STATIC void
 circuit_build_times_reset(circuit_build_times_t *cbt)
 {
   memset(cbt->circuit_build_times, 0, sizeof(cbt->circuit_build_times));
@@ -949,7 +953,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt,
  * an acceptable approximation because we are only concerned with the
  * accuracy of the CDF of the tail.
  */
-int
+STATIC int
 circuit_build_times_update_alpha(circuit_build_times_t *cbt)
 {
   build_time_t *x=cbt->circuit_build_times;
@@ -1033,7 +1037,7 @@ circuit_build_times_update_alpha(circuit_build_times_t *cbt)
  *
  * Return value is in milliseconds.
  */
-double
+STATIC double
 circuit_build_times_calculate_timeout(circuit_build_times_t *cbt,
                                       double quantile)
 {
@@ -1050,6 +1054,7 @@ circuit_build_times_calculate_timeout(circuit_build_times_t *cbt,
   return ret;
 }
 
+#ifdef TOR_UNIT_TESTS
 /** Pareto CDF */
 double
 circuit_build_times_cdf(circuit_build_times_t *cbt, double x)
@@ -1060,7 +1065,9 @@ circuit_build_times_cdf(circuit_build_times_t *cbt, double x)
   tor_assert(0 <= ret && ret <= 1.0);
   return ret;
 }
+#endif
 
+#ifdef TOR_UNIT_TESTS
 /**
  * Generate a synthetic time using our distribution parameters.
  *
@@ -1093,7 +1100,9 @@ circuit_build_times_generate_sample(circuit_build_times_t *cbt,
   tor_assert(ret > 0);
   return ret;
 }
+#endif
 
+#ifdef TOR_UNIT_TESTS
 /**
  * Estimate an initial alpha parameter by solving the quantile
  * function with a quantile point and a specific timeout value.
@@ -1114,6 +1123,7 @@ circuit_build_times_initial_alpha(circuit_build_times_t *cbt,
     (tor_mathlog(cbt->Xm)-tor_mathlog(timeout_ms));
   tor_assert(cbt->alpha > 0);
 }
+#endif
 
 /**
  * Returns true if we need circuits to be built
@@ -1282,7 +1292,7 @@ circuit_build_times_network_check_live(circuit_build_times_t *cbt)
  * to restart the process of building test circuits and estimating a
  * new timeout.
  */
-int
+STATIC int
 circuit_build_times_network_check_changed(circuit_build_times_t *cbt)
 {
   int total_build_times = cbt->total_build_times;
@@ -1546,6 +1556,8 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt)
              cbt->total_build_times);
   }
 }
+
+#ifdef TOR_UNIT_TESTS
 /** Make a note that we're running unit tests (rather than running Tor
  * itself), so we avoid clobbering our state file. */
 void
@@ -1553,4 +1565,5 @@ circuitbuild_running_unit_tests(void)
 {
   unit_tests = 1;
 }
+#endif
 
diff --git a/src/or/circuitstats.h b/src/or/circuitstats.h
index 87dce99f4f..53ba2b2d2e 100644
--- a/src/or/circuitstats.h
+++ b/src/or/circuitstats.h
@@ -38,19 +38,23 @@ double circuit_build_times_timeout_rate(const circuit_build_times_t *cbt);
 double circuit_build_times_close_rate(const circuit_build_times_t *cbt);
 
 #ifdef CIRCUITSTATS_PRIVATE
-double circuit_build_times_calculate_timeout(circuit_build_times_t *cbt,
+STATIC double circuit_build_times_calculate_timeout(circuit_build_times_t *cbt,
                                              double quantile);
-build_time_t circuit_build_times_generate_sample(circuit_build_times_t *cbt,
-                                                 double q_lo, double q_hi);
-void circuit_build_times_initial_alpha(circuit_build_times_t *cbt,
-                                       double quantile, double time_ms);
-int circuit_build_times_update_alpha(circuit_build_times_t *cbt);
-double circuit_build_times_cdf(circuit_build_times_t *cbt, double x);
-void circuitbuild_running_unit_tests(void);
-void circuit_build_times_reset(circuit_build_times_t *cbt);
+STATIC int circuit_build_times_update_alpha(circuit_build_times_t *cbt);
+STATIC void circuit_build_times_reset(circuit_build_times_t *cbt);
 
 /* Network liveness functions */
-int circuit_build_times_network_check_changed(circuit_build_times_t *cbt);
+STATIC int circuit_build_times_network_check_changed(
+                                             circuit_build_times_t *cbt);
+#endif
+
+#ifdef TOR_UNIT_TESTS
+build_time_t circuit_build_times_generate_sample(circuit_build_times_t *cbt,
+                                                 double q_lo, double q_hi);
+double circuit_build_times_cdf(circuit_build_times_t *cbt, double x);
+void circuit_build_times_initial_alpha(circuit_build_times_t *cbt,
+                                       double quantile, double time_ms);
+void circuitbuild_running_unit_tests(void);
 #endif
 
 /* Network liveness functions */
@@ -58,8 +62,5 @@ void circuit_build_times_network_is_live(circuit_build_times_t *cbt);
 int circuit_build_times_network_check_live(circuit_build_times_t *cbt);
 void circuit_build_times_network_circ_success(circuit_build_times_t *cbt);
 
-/* DOCDOC circuit_build_times_get_bw_scale */
-int circuit_build_times_get_bw_scale(networkstatus_t *ns);
-
 #endif
 
diff --git a/src/or/config.c b/src/or/config.c
index a47f349c25..e2f2a3b39e 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -9,8 +9,6 @@
  * \brief Code to parse and interpret configuration files.
  **/
 
-#define CONFIG_PRIVATE
-
 #include "or.h"
 #include "addressmap.h"
 #include "channel.h"
diff --git a/src/or/config.h b/src/or/config.h
index 0250f645d0..7b4cf59da8 100644
--- a/src/or/config.h
+++ b/src/or/config.h
@@ -90,10 +90,7 @@ uint32_t get_effective_bwburst(const or_options_t *options);
 
 char *get_transport_bindaddr_from_config(const char *transport);
 
-#ifdef CONFIG_PRIVATE
-/* Used only by config.c and test.c */
 or_options_t *options_new(void);
-#endif
 
 void config_register_addressmaps(const or_options_t *options);
 /* XXXX024 move to connection_edge.h */
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index bb7ffb9a40..33585a0944 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -407,7 +407,7 @@ connection_edge_finished_flushing(edge_connection_t *conn)
  * that the name resolution that led us to <b>addr</b> will be valid for
  * <b>ttl</b> seconds. Return -1 on error, or the number of bytes used on
  * success. */
-/* private */int
+STATIC int
 connected_cell_format_payload(uint8_t *payload_out,
                               const tor_addr_t *addr,
                               uint32_t ttl)
@@ -2265,7 +2265,7 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply,
  * Return -1 in the case where want to send a RELAY_END cell, and < -1 when
  * we don't.
  **/
-/* static */ int
+STATIC int
 begin_cell_parse(const cell_t *cell, begin_cell_t *bcell,
                  uint8_t *end_reason_out)
 {
diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h
index ea284cbcfd..e3a95ad9ed 100644
--- a/src/or/connection_edge.h
+++ b/src/or/connection_edge.h
@@ -12,6 +12,8 @@
 #ifndef TOR_CONNECTION_EDGE_H
 #define TOR_CONNECTION_EDGE_H
 
+#include "testsupport.h"
+
 #define connection_mark_unattached_ap(conn, endreason) \
   connection_mark_unattached_ap_((conn), (endreason), __LINE__, SHORT_FILE__)
 
@@ -130,9 +132,9 @@ typedef struct begin_cell_t {
   unsigned is_begindir : 1;
 } begin_cell_t;
 
-int begin_cell_parse(const cell_t *cell, begin_cell_t *bcell,
+STATIC int begin_cell_parse(const cell_t *cell, begin_cell_t *bcell,
                      uint8_t *end_reason_out);
-int connected_cell_format_payload(uint8_t *payload_out,
+STATIC int connected_cell_format_payload(uint8_t *payload_out,
                                   const tor_addr_t *addr,
                                   uint32_t ttl);
 #endif
diff --git a/src/or/control.c b/src/or/control.c
index cc917c46aa..3f8d47c554 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -334,7 +334,7 @@ connection_write_str_to_buf(const char *s, control_connection_t *conn)
  * the end. Replace all LF characters sequences with CRLF.  Return the number
  * of bytes in *<b>out</b>.
  */
-/* static */ size_t
+STATIC size_t
 write_escaped_data(const char *data, size_t len, char **out)
 {
   size_t sz_out = len+8;
@@ -382,7 +382,7 @@ write_escaped_data(const char *data, size_t len, char **out)
  * that appears at the start of a line, and replacing all CRLF sequences
  * with LF.   Return the number of
  * bytes in *<b>out</b>. */
-/* static */ size_t
+STATIC size_t
 read_escaped_data(const char *data, size_t len, char **out)
 {
   char *outp;
diff --git a/src/or/control.h b/src/or/control.h
index 61062da2c4..d0f682067e 100644
--- a/src/or/control.h
+++ b/src/or/control.h
@@ -91,8 +91,8 @@ void control_event_clients_seen(const char *controller_str);
 
 #ifdef CONTROL_PRIVATE
 /* Used only by control.c and test.c */
-size_t write_escaped_data(const char *data, size_t len, char **out);
-size_t read_escaped_data(const char *data, size_t len, char **out);
+STATIC size_t write_escaped_data(const char *data, size_t len, char **out);
+STATIC size_t read_escaped_data(const char *data, size_t len, char **out);
 #endif
 
 #endif
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 97fe068848..3243ac47c4 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -2082,7 +2082,7 @@ static digestmap_t *mbw_cache = NULL;
 
 /** Store a measured bandwidth cache entry when reading the measured
  * bandwidths file. */
-void
+STATIC void
 dirserv_cache_measured_bw(const measured_bw_line_t *parsed_line,
                           time_t as_of)
 {
@@ -2112,7 +2112,7 @@ dirserv_cache_measured_bw(const measured_bw_line_t *parsed_line,
 }
 
 /** Clear and free the measured bandwidth cache */
-void
+STATIC void
 dirserv_clear_measured_bw_cache(void)
 {
   if (mbw_cache) {
@@ -2123,7 +2123,7 @@ dirserv_clear_measured_bw_cache(void)
 }
 
 /** Scan the measured bandwidth cache and remove expired entries */
-void
+STATIC void
 dirserv_expire_measured_bw_cache(time_t now)
 {
 
@@ -2145,7 +2145,7 @@ dirserv_expire_measured_bw_cache(time_t now)
 }
 
 /** Get the current size of the measured bandwidth cache */
-int
+STATIC int
 dirserv_get_measured_bw_cache_size(void)
 {
   if (mbw_cache) return digestmap_size(mbw_cache);
@@ -2155,7 +2155,7 @@ dirserv_get_measured_bw_cache_size(void)
 /** Query the cache by identity digest, return value indicates whether
  * we found it. The bw_out and as_of_out pointers receive the cached
  * bandwidth value and the time it was cached if not NULL. */
-int
+STATIC int
 dirserv_query_measured_bw_cache_kb(const char *node_id, long *bw_kb_out,
                                    time_t *as_of_out)
 {
@@ -2176,7 +2176,7 @@ dirserv_query_measured_bw_cache_kb(const char *node_id, long *bw_kb_out,
 }
 
 /** Predicate wrapper for dirserv_query_measured_bw_cache() */
-int
+STATIC int
 dirserv_has_measured_bw(const char *node_id)
 {
   return dirserv_query_measured_bw_cache_kb(node_id, NULL, NULL);
@@ -2754,7 +2754,7 @@ clear_status_flags_on_sybil(routerstatus_t *rs)
  * into a measured_bw_line_t output structure. Returns -1 on failure
  * or 0 on success.
  */
-int
+STATIC int
 measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line)
 {
   char *line = tor_strdup(orig_line);
@@ -2835,7 +2835,7 @@ measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line)
  * of bandwidth statuses. Returns true if a line is found,
  * false otherwise.
  */
-int
+STATIC int
 measured_bw_line_apply(measured_bw_line_t *parsed_line,
                        smartlist_t *routerstatuses)
 {
@@ -3168,7 +3168,7 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_t *private_key,
 /** For v2 authoritative directories only: Replace the contents of
  * <b>the_v2_networkstatus</b> with a newly generated network status
  * object.  */
-cached_dir_t *
+STATIC cached_dir_t *
 generate_v2_networkstatus_opinion(void)
 {
   cached_dir_t *r = NULL;
diff --git a/src/or/dirserv.h b/src/or/dirserv.h
index f9d36d760f..7221fc9957 100644
--- a/src/or/dirserv.h
+++ b/src/or/dirserv.h
@@ -12,6 +12,8 @@
 #ifndef TOR_DIRSERV_H
 #define TOR_DIRSERV_H
 
+#include "testsupport.h"
+
 /** What fraction (1 over this number) of the relay ID space do we
  * (as a directory authority) launch connections to at each reachability
  * test? */
@@ -119,20 +121,21 @@ cached_dir_t *new_cached_dir(char *s, time_t published);
 /* Put the MAX_MEASUREMENT_AGE #define here so unit tests can see it */
 #define MAX_MEASUREMENT_AGE (3*24*60*60) /* 3 days */
 
-int measured_bw_line_parse(measured_bw_line_t *out, const char *line);
+STATIC int measured_bw_line_parse(measured_bw_line_t *out, const char *line);
 
-int measured_bw_line_apply(measured_bw_line_t *parsed_line,
+STATIC int measured_bw_line_apply(measured_bw_line_t *parsed_line,
                            smartlist_t *routerstatuses);
 
-void dirserv_cache_measured_bw(const measured_bw_line_t *parsed_line,
+STATIC void dirserv_cache_measured_bw(const measured_bw_line_t *parsed_line,
                                time_t as_of);
-void dirserv_clear_measured_bw_cache(void);
-void dirserv_expire_measured_bw_cache(time_t now);
-int dirserv_get_measured_bw_cache_size(void);
-int dirserv_query_measured_bw_cache_kb(const char *node_id, long *bw_out,
-                                       time_t *as_of_out);
-int dirserv_has_measured_bw(const char *node_id);
-cached_dir_t *generate_v2_networkstatus_opinion(void);
+STATIC void dirserv_clear_measured_bw_cache(void);
+STATIC void dirserv_expire_measured_bw_cache(time_t now);
+STATIC int dirserv_get_measured_bw_cache_size(void);
+STATIC int dirserv_query_measured_bw_cache_kb(const char *node_id,
+                                              long *bw_out,
+                                              time_t *as_of_out);
+STATIC int dirserv_has_measured_bw(const char *node_id);
+STATIC cached_dir_t *generate_v2_networkstatus_opinion(void);
 #endif
 
 int dirserv_read_measured_bandwidths(const char *from_file,
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index f65a26b660..12ceba8549 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -60,7 +60,7 @@ static char *make_consensus_method_list(int low, int high, const char *sep);
 /** Return a new string containing the string representation of the vote in
  * <b>v3_ns</b>, signed with our v3 signing key <b>private_signing_key</b>.
  * For v3 authorities. */
-char *
+STATIC char *
 format_networkstatus_vote(crypto_pk_t *private_signing_key,
                           networkstatus_t *v3_ns)
 {
@@ -587,7 +587,7 @@ compute_consensus_versions_list(smartlist_t *lst, int n_versioning)
 /** Helper: given a list of valid networkstatus_t, return a new string
  * containing the contents of the consensus network parameter set.
  */
-/* private */ char *
+STATIC char *
 dirvote_compute_params(smartlist_t *votes, int method, int total_authorities)
 {
   int i;
diff --git a/src/or/dirvote.h b/src/or/dirvote.h
index a3e6cc0d68..3a4951a95f 100644
--- a/src/or/dirvote.h
+++ b/src/or/dirvote.h
@@ -12,6 +12,8 @@
 #ifndef TOR_DIRVOTE_H
 #define TOR_DIRVOTE_H
 
+#include "testsupport.h"
+
 /** Lowest allowable value for VoteSeconds. */
 #define MIN_VOTE_SECONDS 2
 /** Lowest allowable value for DistSeconds. */
@@ -136,9 +138,9 @@ document_signature_t *voter_get_sig_by_algorithm(
                            digest_algorithm_t alg);
 
 #ifdef DIRVOTE_PRIVATE
-char *format_networkstatus_vote(crypto_pk_t *private_key,
+STATIC char *format_networkstatus_vote(crypto_pk_t *private_key,
                                  networkstatus_t *v3_ns);
-char *dirvote_compute_params(smartlist_t *votes, int method,
+STATIC char *dirvote_compute_params(smartlist_t *votes, int method,
                              int total_authorities);
 #endif
 
diff --git a/src/or/geoip.c b/src/or/geoip.c
index d6e8ee0d06..020075f80f 100644
--- a/src/or/geoip.c
+++ b/src/or/geoip.c
@@ -120,7 +120,7 @@ geoip_add_entry(const tor_addr_t *low, const tor_addr_t *high,
 
 /** Add an entry to the GeoIP table indicated by <b>family</b>,
  * parsing it from <b>line</b>. The format is as for geoip_load_file(). */
-/*private*/ int
+STATIC int
 geoip_parse_entry(const char *line, sa_family_t family)
 {
   tor_addr_t low_addr, high_addr;
@@ -363,7 +363,7 @@ geoip_load_file(sa_family_t family, const char *filename)
  * be less than geoip_get_n_countries().  To decode it, call
  * geoip_get_country_name().
  */
-int
+STATIC int
 geoip_get_country_by_ipv4(uint32_t ipaddr)
 {
   geoip_ipv4_entry_t *ent;
@@ -379,7 +379,7 @@ geoip_get_country_by_ipv4(uint32_t ipaddr)
  * 0 for the 'unknown country'.  The return value will always be less than
  * geoip_get_n_countries().  To decode it, call geoip_get_country_name().
  */
-int
+STATIC int
 geoip_get_country_by_ipv6(const struct in6_addr *addr)
 {
   geoip_ipv6_entry_t *ent;
diff --git a/src/or/geoip.h b/src/or/geoip.h
index ebefee5f4e..d67007298e 100644
--- a/src/or/geoip.h
+++ b/src/or/geoip.h
@@ -12,10 +12,12 @@
 #ifndef TOR_GEOIP_H
 #define TOR_GEOIP_H
 
+#include "testsupport.h"
+
 #ifdef GEOIP_PRIVATE
-int geoip_parse_entry(const char *line, sa_family_t family);
-int geoip_get_country_by_ipv4(uint32_t ipaddr);
-int geoip_get_country_by_ipv6(const struct in6_addr *addr);
+STATIC int geoip_parse_entry(const char *line, sa_family_t family);
+STATIC int geoip_get_country_by_ipv4(uint32_t ipaddr);
+STATIC int geoip_get_country_by_ipv6(const struct in6_addr *addr);
 #endif
 int should_record_bridge_info(const or_options_t *options);
 int geoip_load_file(sa_family_t family, const char *filename);
diff --git a/src/or/hibernate.c b/src/or/hibernate.c
index a412571331..22c0f253da 100644
--- a/src/or/hibernate.c
+++ b/src/or/hibernate.c
@@ -1010,6 +1010,7 @@ getinfo_helper_accounting(control_connection_t *conn,
   return 0;
 }
 
+#ifdef TOR_UNIT_TESTS
 /**
  * Manually change the hibernation state.  Private; used only by the unit
  * tests.
@@ -1019,4 +1020,5 @@ hibernate_set_state_for_testing_(hibernate_state_t newstate)
 {
   hibernate_state = newstate;
 }
+#endif
 
diff --git a/src/or/hibernate.h b/src/or/hibernate.h
index d2d6989e10..18832fbc6c 100644
--- a/src/or/hibernate.h
+++ b/src/or/hibernate.h
@@ -45,8 +45,10 @@ typedef enum {
   HIBERNATE_STATE_INITIAL=5
 } hibernate_state_t;
 
+#ifdef TOR_UNIT_TESTS
 void hibernate_set_state_for_testing_(hibernate_state_t newstate);
 #endif
+#endif
 
 #endif
 
diff --git a/src/or/main.c b/src/or/main.c
index 85aa97f5a1..21133855a0 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -10,7 +10,6 @@
  * connections, implements main loop, and drives scheduled events.
  **/
 
-#define MAIN_PRIVATE
 #include "or.h"
 #include "addressmap.h"
 #include "buffers.h"
@@ -1874,7 +1873,7 @@ do_hup(void)
 }
 
 /** Tor main loop. */
-/* static */ int
+int
 do_main_loop(void)
 {
   int loop_result;
@@ -2301,7 +2300,7 @@ handle_signals(int is_parent)
 
 /** Main entry point for the Tor command-line client.
  */
-/* static */ int
+int
 tor_init(int argc, char *argv[])
 {
   char buf[256];
@@ -2567,7 +2566,7 @@ tor_cleanup(void)
 }
 
 /** Read/create keys as needed, and echo our fingerprint to stdout. */
-/* static */ int
+static int
 do_list_fingerprint(void)
 {
   char buf[FINGERPRINT_LEN+1];
@@ -2597,7 +2596,7 @@ do_list_fingerprint(void)
 
 /** Entry point for password hashing: take the desired password from
  * the command line, and print its salted hash to stdout. **/
-/* static */ void
+static void
 do_hash_password(void)
 {
 
diff --git a/src/or/main.h b/src/or/main.h
index 338449b6a6..85621a32bf 100644
--- a/src/or/main.h
+++ b/src/or/main.h
@@ -66,12 +66,8 @@ void tor_free_all(int postfork);
 
 int tor_main(int argc, char *argv[]);
 
-#ifdef MAIN_PRIVATE
 int do_main_loop(void);
-int do_list_fingerprint(void);
-void do_hash_password(void);
 int tor_init(int argc, char **argv);
-#endif
 
 #endif
 
diff --git a/src/or/ntmain.c b/src/or/ntmain.c
index 8b67b86822..2fa074d0be 100644
--- a/src/or/ntmain.c
+++ b/src/or/ntmain.c
@@ -3,7 +3,6 @@
  * Copyright (c) 2007-2013, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
-#define MAIN_PRIVATE
 #include "or.h"
 #include "config.h"
 #include "main.h"
diff --git a/src/or/relay.c b/src/or/relay.c
index 251d3ffa2f..7ec7e03024 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -969,7 +969,7 @@ remap_event_helper(entry_connection_t *conn, const tor_addr_t *new_addr)
  * <b>addr_out</b> to the address we're connected to, and <b>ttl_out</b> to
  * the ttl of that address, in seconds, and return 0.  On failure, return
  * -1. */
-int
+STATIC int
 connected_cell_parse(const relay_header_t *rh, const cell_t *cell,
                      tor_addr_t *addr_out, int *ttl_out)
 {
diff --git a/src/or/relay.h b/src/or/relay.h
index 69df7024b3..4be3194e5b 100644
--- a/src/or/relay.h
+++ b/src/or/relay.h
@@ -75,10 +75,11 @@ void circuit_clear_cell_queue(circuit_t *circ, channel_t *chan);
 
 void stream_choice_seed_weak_rng(void);
 
-#ifdef RELAY_PRIVATE
 int relay_crypt(circuit_t *circ, cell_t *cell, cell_direction_t cell_direction,
                 crypt_path_t **layer_hint, char *recognized);
-int connected_cell_parse(const relay_header_t *rh, const cell_t *cell,
+
+#ifdef RELAY_PRIVATE
+STATIC int connected_cell_parse(const relay_header_t *rh, const cell_t *cell,
                          tor_addr_t *addr_out, int *ttl_out);
 #endif
 
diff --git a/src/or/replaycache.c b/src/or/replaycache.c
index 59b98489b7..f136072f61 100644
--- a/src/or/replaycache.c
+++ b/src/or/replaycache.c
@@ -63,7 +63,7 @@ replaycache_new(time_t horizon, time_t interval)
 /** See documentation for replaycache_add_and_test()
  */
 
-int
+STATIC int
 replaycache_add_and_test_internal(
     time_t present, replaycache_t *r, const void *data, int len,
     time_t *elapsed)
@@ -127,7 +127,7 @@ replaycache_add_and_test_internal(
 /** See documentation for replaycache_scrub_if_needed()
  */
 
-void
+STATIC void
 replaycache_scrub_if_needed_internal(time_t present, replaycache_t *r)
 {
   digestmap_iter_t *itr = NULL;
diff --git a/src/or/replaycache.h b/src/or/replaycache.h
index de20cab627..c60c408103 100644
--- a/src/or/replaycache.h
+++ b/src/or/replaycache.h
@@ -45,10 +45,10 @@ replaycache_t * replaycache_new(time_t horizon, time_t interval);
  * testing.  For everything else, use the wrappers below instead.
  */
 
-int replaycache_add_and_test_internal(
+STATIC int replaycache_add_and_test_internal(
     time_t present, replaycache_t *r, const void *data, int len,
     time_t *elapsed);
-void replaycache_scrub_if_needed_internal(
+STATIC void replaycache_scrub_if_needed_internal(
     time_t present, replaycache_t *r);
 
 #endif /* REPLAYCACHE_PRIVATE */
diff --git a/src/or/router.c b/src/or/router.c
index 6069da8f09..1063eda044 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -2249,7 +2249,7 @@ router_guess_address_from_dir_headers(uint32_t *guess)
  * string describing the version of Tor and the operating system we're
  * currently running on.
  */
-void
+STATIC void
 get_platform_str(char *platform, size_t len)
 {
   tor_snprintf(platform, len, "Tor %s on %s",
diff --git a/src/or/router.h b/src/or/router.h
index 60095d087b..1079ec78c2 100644
--- a/src/or/router.h
+++ b/src/or/router.h
@@ -12,6 +12,8 @@
 #ifndef TOR_ROUTER_H
 #define TOR_ROUTER_H
 
+#include "testsupport.h"
+
 crypto_pk_t *get_onion_key(void);
 time_t get_onion_key_set_at(void);
 void set_server_identity_key(crypto_pk_t *k);
@@ -146,7 +148,7 @@ smartlist_t *router_get_all_orports(const routerinfo_t *ri);
 
 #ifdef ROUTER_PRIVATE
 /* Used only by router.c and test.c */
-void get_platform_str(char *platform, size_t len);
+STATIC void get_platform_str(char *platform, size_t len);
 #endif
 
 #endif
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index a145ba716e..46da17e03b 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1801,7 +1801,7 @@ router_get_advertised_bandwidth_capped(const routerinfo_t *router)
  * doubles, convert them to uint64_t, and try to scale them linearly so as to
  * much of the range of uint64_t. If <b>total_out</b> is provided, set it to
  * the sum of all elements in the array _before_ scaling. */
-/* private */ void
+STATIC void
 scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
                             uint64_t *total_out)
 {
@@ -1844,7 +1844,7 @@ gt_i64_timei(uint64_t a, uint64_t b)
  * value, and return the index of that element.  If all elements are 0, choose
  * an index at random. Return -1 on error.
  */
-/* private */ int
+STATIC int
 choose_array_element_by_weight(const u64_dbl_t *entries, int n_entries)
 {
   int i, i_chosen=-1, n_chosen=0;
diff --git a/src/or/routerlist.h b/src/or/routerlist.h
index ce0f0f2e34..0162297ca7 100644
--- a/src/or/routerlist.h
+++ b/src/or/routerlist.h
@@ -11,6 +11,8 @@
 #ifndef TOR_ROUTERLIST_H
 #define TOR_ROUTERLIST_H
 
+#include "testsupport.h"
+
 int get_n_authorities(dirinfo_type_t type);
 int trusted_dirs_reload_certs(void);
 
@@ -206,9 +208,10 @@ typedef union u64_dbl_t {
   double dbl;
 } u64_dbl_t;
 
-int choose_array_element_by_weight(const u64_dbl_t *entries, int n_entries);
-void scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
-                                 uint64_t *total_out);
+STATIC int choose_array_element_by_weight(const u64_dbl_t *entries,
+                                          int n_entries);
+STATIC void scale_array_elements_to_u64(u64_dbl_t *entries, int n_entries,
+                                        uint64_t *total_out);
 #endif
 
 #endif
diff --git a/src/or/transports.c b/src/or/transports.c
index 0afba24ea0..cfec70340c 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -776,7 +776,7 @@ handle_methods_done(const managed_proxy_t *mp)
 
 /** Handle a configuration protocol <b>line</b> received from a
  *  managed proxy <b>mp</b>. */
-void
+STATIC void
 handle_proxy_line(const char *line, managed_proxy_t *mp)
 {
   log_info(LD_GENERAL, "Got a line from managed proxy '%s': (%s)",
@@ -877,7 +877,7 @@ handle_proxy_line(const char *line, managed_proxy_t *mp)
 }
 
 /** Parses an ENV-ERROR <b>line</b> and warns the user accordingly. */
-void
+STATIC void
 parse_env_error(const char *line)
 {
   /* (Length of the protocol string) plus (a space) and (the first char of
@@ -893,7 +893,7 @@ parse_env_error(const char *line)
 
 /** Handles a VERSION <b>line</b>. Updates the configuration protocol
  *  version in <b>mp</b>. */
-int
+STATIC int
 parse_version(const char *line, managed_proxy_t *mp)
 {
   if (strlen(line) < (strlen(PROTO_NEG_SUCCESS) + 2)) {
@@ -934,7 +934,7 @@ parse_method_error(const char *line, int is_server)
 
 /** Parses an SMETHOD <b>line</b> and if well-formed it registers the
  *  new transport in <b>mp</b>. */
-int
+STATIC int
 parse_smethod_line(const char *line, managed_proxy_t *mp)
 {
   int r;
@@ -1011,7 +1011,7 @@ parse_smethod_line(const char *line, managed_proxy_t *mp)
 
 /** Parses a CMETHOD <b>line</b>, and if well-formed it registers
  *  the new transport in <b>mp</b>. */
-int
+STATIC int
 parse_cmethod_line(const char *line, managed_proxy_t *mp)
 {
   int r;
diff --git a/src/or/transports.h b/src/or/transports.h
index 4a5498cb58..cc3e018d6d 100644
--- a/src/or/transports.h
+++ b/src/or/transports.h
@@ -104,12 +104,12 @@ typedef struct {
   smartlist_t *transports;
 } managed_proxy_t;
 
-int parse_cmethod_line(const char *line, managed_proxy_t *mp);
-int parse_smethod_line(const char *line, managed_proxy_t *mp);
+STATIC int parse_cmethod_line(const char *line, managed_proxy_t *mp);
+STATIC int parse_smethod_line(const char *line, managed_proxy_t *mp);
 
-int parse_version(const char *line, managed_proxy_t *mp);
-void parse_env_error(const char *line);
-void handle_proxy_line(const char *line, managed_proxy_t *mp);
+STATIC int parse_version(const char *line, managed_proxy_t *mp);
+STATIC void parse_env_error(const char *line);
+STATIC void handle_proxy_line(const char *line, managed_proxy_t *mp);
 
 #endif
 
diff --git a/src/test/bench.c b/src/test/bench.c
index 97a7909fc9..ca01d3c3e6 100644
--- a/src/test/bench.c
+++ b/src/test/bench.c
@@ -15,7 +15,6 @@ const char tor_git_revision[] = "";
 #include "orconfig.h"
 
 #define RELAY_PRIVATE
-#define CONFIG_PRIVATE
 
 #include "or.h"
 #include "onion_tap.h"
diff --git a/src/test/test.c b/src/test/test.c
index a9cf899a0e..7721d2c730 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -29,7 +29,6 @@ const char tor_git_revision[] = "";
 /* These macros pull in declarations for some functions and structures that
  * are typically file-private. */
 #define BUFFERS_PRIVATE
-#define CONFIG_PRIVATE
 #define GEOIP_PRIVATE
 #define ROUTER_PRIVATE
 #define CIRCUITSTATS_PRIVATE
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index f391cce6e6..9dc43b1d27 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -4,7 +4,6 @@
 /* See LICENSE for licensing information */
 
 #include "orconfig.h"
-#define CRYPTO_PRIVATE
 #define CRYPTO_CURVE25519_PRIVATE
 #include "or.h"
 #include "test.h"
@@ -632,7 +631,7 @@ test_crypto_formats(void)
     data1 = tor_strdup("ABCD1234ABCD56780000ABCD1234ABCD56780000");
     test_eq(strlen(data1), 40);
     data2 = tor_malloc(FINGERPRINT_LEN+1);
-    add_spaces_to_fp(data2, FINGERPRINT_LEN+1, data1);
+    crypto_add_spaces_to_fp(data2, FINGERPRINT_LEN+1, data1);
     test_streq(data2, "ABCD 1234 ABCD 5678 0000 ABCD 1234 ABCD 5678 0000");
     tor_free(data1);
     tor_free(data2);
diff --git a/src/tools/tor-checkkey.c b/src/tools/tor-checkkey.c
index a3860ca4b7..d50f12ed2a 100644
--- a/src/tools/tor-checkkey.c
+++ b/src/tools/tor-checkkey.c
@@ -1,8 +1,6 @@
 /* Copyright (c) 2008-2013, The Tor Project, Inc. */
 /* See LICENSE for licensing information */
 
-#define CRYPTO_PRIVATE
-
 #include "orconfig.h"
 
 #include <stdio.h>
diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c
index 3809b22d43..25beb2aae1 100644
--- a/src/tools/tor-gencert.c
+++ b/src/tools/tor-gencert.c
@@ -27,8 +27,6 @@
 #include <assert.h>
 #endif
 
-#define CRYPTO_PRIVATE
-
 #include "compat.h"
 #include "../common/util.h"
 #include "../common/torlog.h"

From 17e9fc09c31dffdc647385220a14daef5a35273a Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 9 Jul 2013 12:37:11 -0400
Subject: [PATCH 3/6] Coverage support: build with --enable-coverage to have
 tests run with gcov

If you pass the --enable-coverage flag on the command line, we build
our testing binaries with appropriate options eo enable coverage
testing.  We also build a "tor-cov" binary that has coverage enabled,
for integration tests.

On recent OSX versions, test coverage only works with clang, not gcc.
So we warn about that.

Also add a contrib/coverage script to actually run gcov with the
appropriate options to generate useful .gcov files.  (Thanks to
automake, the .o files will not have the names that gcov expects to
find.)

Also, remove generated gcda and gcno files on clean.
---
 .gitignore            |  2 ++
 Makefile.am           |  8 ++++++++
 changes/fancy_testing |  4 ++++
 configure.ac          | 14 ++++++++++++--
 contrib/coverage      | 13 +++++++++++++
 src/common/include.am |  4 ++++
 src/or/include.am     | 16 ++++++++++++++++
 src/test/include.am   |  2 ++
 8 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100755 contrib/coverage

diff --git a/.gitignore b/.gitignore
index bb3817ba57..201d04da66 100644
--- a/.gitignore
+++ b/.gitignore
@@ -152,6 +152,8 @@
 /src/or/or_sha1.i
 /src/or/tor
 /src/or/tor.exe
+/src/or/tor-cov
+/src/or/tor-cov.exe
 /src/or/libtor.a
 /src/or/libtor-testing.a
 /src/or/libtor.lib
diff --git a/Makefile.am b/Makefile.am
index a736638e4b..1bc1077ebb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -32,6 +32,12 @@ EXTRA_DIST+= \
 	README						\
 	ReleaseNotes
 
+if COVERAGE_ENABLED
+TEST_CFLAGS=-fno-inline -fprofile-arcs -ftest-coverage
+else
+TEST_CFLAGS=
+endif
+
 #install-data-local:
 #	$(INSTALL) -m 755 -d $(LOCALSTATEDIR)/lib/tor
 
@@ -89,3 +95,5 @@ version:
 	   (cd "$(top_srcdir)" && git rev-parse --short=16 HEAD); \
 	fi
 
+mostlyclean-local:
+	rm -f src/*/*.gc{da,no}
diff --git a/changes/fancy_testing b/changes/fancy_testing
index 3e8ccbca48..ad197c6f58 100644
--- a/changes/fancy_testing
+++ b/changes/fancy_testing
@@ -10,3 +10,7 @@
       in the unit tests; all functions exposed from a module for
       unit-testing only are now static in production builds.
 
+    - Add an --enable-coverage configuration option to make the unit
+      tests (and a new src/or/tor-cov target) to build with gcov test
+      coverage support.
+
diff --git a/configure.ac b/configure.ac
index 34ed524dd8..22aac31bf2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -41,8 +41,13 @@ AC_ARG_ENABLE(curve25519,
    AS_HELP_STRING(--disable-curve25519, Build Tor with no curve25519 elliptic-curve crypto support))
 AC_ARG_ENABLE(unittests,
    AS_HELP_STRING(--disable-unittests, [Don't build unit tests for Tor. Risky!]))
+AC_ARG_ENABLE(coverage,
+   AS_HELP_STRING(--enable-coverage, [Enable coverage support in the unit-test build]))
 
-AM_CONDITIONAL(UNITTESTS_ENABLED, test x$unittests != xno)
+AM_CONDITIONAL(UNITTESTS_ENABLED, test x$enable_unittests != xno)
+AM_CONDITIONAL(COVERAGE_ENABLED, test x$enable_coverage = xyes)
+
+echo "COVERAGE: $enable_coverage"
 
 if test "$enable_static_tor" = "yes"; then
   enable_static_libevent="yes";
@@ -1457,7 +1462,12 @@ if test x$enable_gcc_warnings = xyes || test x$enable_gcc_warnings_advisory = xy
 # CFLAGS="$CFLAGS -Winline"
 fi
 
-
+if test "$enable_coverage" = yes && test "$have_clang" = "no"; then
+   case "$host_os" in
+    darwin*)
+      AC_MSG_WARN([Tried to enable coverage on OSX without using the clang compiler. This might not work! If coverage fails, use CC=clang when configuring with --enable-profiling.])
+   esac
+fi
 
 CPPFLAGS="$CPPFLAGS $TOR_CPPFLAGS_libevent $TOR_CPPFLAGS_openssl $TOR_CPPFLAGS_zlib"
 
diff --git a/contrib/coverage b/contrib/coverage
new file mode 100755
index 0000000000..52fe96d756
--- /dev/null
+++ b/contrib/coverage
@@ -0,0 +1,13 @@
+#!/bin/sh
+# Copyright 2013  The Tor Project, Inc.
+# See LICENSE for licensing information.
+
+# coverage -- run gcov on the appropriate set of object files to extract
+# coverage information.
+
+for fn in src/{or,common}/*.c; do
+    BN=`basename $fn`
+    DN=`dirname $fn`
+    F=`echo $BN | sed -e 's/\.c$//;'`
+    gcov -o $DN/src_*$F.o $fn
+done
diff --git a/src/common/include.am b/src/common/include.am
index 3d8cc8e9eb..3f5a0e0d0e 100644
--- a/src/common/include.am
+++ b/src/common/include.am
@@ -82,6 +82,10 @@ src_common_libor_event_testing_a_SOURCES = $(LIBOR_EVENT_A_SOURCES)
 src_common_libor_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
 src_common_libor_crypto_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
 src_common_libor_event_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
+src_common_libor_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
+src_common_libor_crypto_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
+src_common_libor_event_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
+
 
 COMMONHEADERS = \
   src/common/address.h				\
diff --git a/src/or/include.am b/src/or/include.am
index 91b9bfce72..f5002e6986 100644
--- a/src/or/include.am
+++ b/src/or/include.am
@@ -5,6 +5,9 @@ if UNITTESTS_ENABLED
 noinst_LIBRARIES += \
 	src/or/libtor-testing.a
 endif
+if COVERAGE_ENABLED
+noinst_PROGRAMS+= src/or/tor-cov
+endif
 
 if BUILD_NT_SERVICES
 tor_platform_source=src/or/ntmain.c
@@ -99,6 +102,7 @@ AM_CPPFLAGS += -DSHARE_DATADIR="\"$(datadir)\"" \
         -DBINDIR="\"$(bindir)\""
 
 src_or_libtor_testing_a_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
+src_or_libtor_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
 
 # -L flags need to go in LDFLAGS. -l flags need to go in LDADD.
 # This seems to matter nowhere but on windows, but I assure you that it
@@ -112,6 +116,18 @@ src_or_tor_LDADD = src/or/libtor.a src/common/libor.a \
 	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ @TOR_OPENSSL_LIBS@ \
 	@TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@
 
+if COVERAGE_ENABLED
+src_or_tor_cov_SOURCES = src/or/tor_main.c
+src_or_tor_cov_CPPFLAGS = -DTOR_UNIT_TESTS $(AM_CPPFLAGS)
+src_or_tor_cov_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
+src_or_tor_cov_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ @TOR_LDFLAGS_libevent@
+src_or_tor_cov_LDADD = src/or/libtor-testing.a src/common/libor-testing.a \
+	src/common/libor-crypto-testing.a $(LIBDONNA) \
+	src/common/libor-event-testing.a \
+	@TOR_ZLIB_LIBS@ @TOR_LIB_MATH@ @TOR_LIBEVENT_LIBS@ @TOR_OPENSSL_LIBS@ \
+	@TOR_LIB_WS32@ @TOR_LIB_GDI@ @CURVE25519_LIBS@
+endif
+
 ORHEADERS = \
 	src/or/addressmap.h				\
 	src/or/buffers.h				\
diff --git a/src/test/include.am b/src/test/include.am
index 08eb7fba67..989cf4ebfc 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -31,6 +31,8 @@ src_test_test_SOURCES = \
 	src/test/test_config.c \
 	src/ext/tinytest.c
 
+src_test_test_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS)
+
 src_test_test_CPPFLAGS= $(src_test_AM_CPPFLAGS)
 
 src_test_bench_SOURCES = \

From 4753ad4f1dbd7fa3233ead5770e9c8bd619b8d07 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 9 Jul 2013 12:54:39 -0400
Subject: [PATCH 4/6] Add a script to compare directories full of gcov output

We can't just use "diff", since we don't care about changes in line
numbers, or changes in the exact number of times a line was called.
We just care about changes that make lines covered or non-coverd.  So
pre-process the files before calling diff.
---
 contrib/cov-diff | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100755 contrib/cov-diff

diff --git a/contrib/cov-diff b/contrib/cov-diff
new file mode 100755
index 0000000000..33a54802b6
--- /dev/null
+++ b/contrib/cov-diff
@@ -0,0 +1,17 @@
+#!/bin/sh
+# Copyright 2013  The Tor Project, Inc.
+# See LICENSE for licensing information.
+
+# cov-diff -- compare two directories full of gcov files.
+
+DIRA="$1"
+DIRB="$2"
+
+for A in $DIRA/*; do
+  B=$DIRB/`basename $A`
+  perl -pe 's/^\s*\d+:/        1:/; s/^([^:]+:)[\d\s]+:/$1/;' "$A" > "$A.tmp"
+  perl -pe 's/^\s*\d+:/        1:/; s/^([^:]+:)[\d\s]+:/$1/;' "$B" > "$B.tmp"
+  diff -u "$A.tmp" "$B.tmp"
+  rm "$A.tmp" "$B.tmp"
+done
+

From b6e8c74667cf723c5ef4d081fc901752e05f9a9b Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 10 Jul 2013 15:03:01 -0400
Subject: [PATCH 5/6] Add rudimentary test mocking support.

This is not the most beautiful possible implementation (it requires
decorating mockable functions with ugly macros), but it actually
works, and is portable across multiple compilers and architectures.
---
 changes/fancy_testing    | 12 ++++++++
 src/common/testsupport.h | 66 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/changes/fancy_testing b/changes/fancy_testing
index ad197c6f58..89876ff270 100644
--- a/changes/fancy_testing
+++ b/changes/fancy_testing
@@ -14,3 +14,15 @@
       tests (and a new src/or/tor-cov target) to build with gcov test
       coverage support.
 
+  o Testing:
+
+    - We now have rudimentary function mocking support that our unit
+      tests can use to test functions in isolation. Function mocking
+      lets the tests temporarily replace a function's dependencies with
+      stub functions, so that the tests can check the function without
+      invoking the other functions it calls.
+
+
+
+
+
diff --git a/src/common/testsupport.h b/src/common/testsupport.h
index c6777d0cb2..4a4f50b69b 100644
--- a/src/common/testsupport.h
+++ b/src/common/testsupport.h
@@ -10,5 +10,71 @@
 #define STATIC static
 #endif
 
+/** Quick and dirty macros to implement test mocking.
+ *
+ * To use them, suppose that you have a function you'd like to mock
+ * with the signature "void writebuf(size_t n, char *buf)".  You can then
+ * declare the function as:
+ *
+ *     MOCK_DECL(void, writebuf, (size_t n, char *buf));
+ *
+ * and implement it as:
+ *
+ *     MOCK_IMPL(void
+ *     writebuf,(size_t n, char *buf)
+ *     {
+ *          ...
+ *     }
+ *
+ * For the non-testing build, this will expand simply into:
+ *
+ *     void writebuf(size_t n, char *buf);
+ *     void
+ *     writebuf(size_t n, char *buf)
+ *     {
+ *         ...
+ *     }
+ *
+ * But for the testing case, it will expand into:
+ *
+ *     void writebuf__real(size_t n, char *buf);
+ *     extern void (*writebuf)(size_t n, char *buf);
+ *
+ *     void (*writebuf)(size_t n, char *buf) = writebuf__real;
+ *     void
+ *     writebuf__real(size_t n, char *buf)
+ *     {
+ *         ...
+ *     }
+ *
+ * This is not a great mocking system!  It is deliberately "the simplest
+ * thing that could work", and pays for its simplicity in its lack of
+ * features, and in its uglification of the Tor code.  Replacing it with
+ * something clever would be a fine thing.
+ *
+ * @{ */
+#ifdef TOR_UNIT_TESTS
+#define MOCK_DECL(rv, funcname, arglist)     \
+  rv funcname ##__real arglist;              \
+  extern rv(*funcname) arglist
+#define MOCK_IMPL(rv, funcname, arglist)     \
+  rv(*funcname) arglist = funcname ##__real; \
+  rv funcname ##__real arglist
+#define MOCK(func, replacement)                 \
+  do {                                          \
+    (func) = (replacement);                     \
+  } while (0)
+#define UNMOCK(func)                            \
+  do {                                          \
+    func = func ##__real;                       \
+  } while (0)
+#else
+#define MOCK_DECL(rv, funcname, arglist) \
+  rv funcname arglist
+#define MOCK_IMPL(rv, funcname, arglist) \
+  rv funcname arglist
+#endif
+/** @} */
+
 #endif
 

From ec6c155f827000e337796f1f1c54299fbc5cf72a Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Wed, 10 Jul 2013 15:07:32 -0400
Subject: [PATCH 6/6] Add some basic unit tests for the circuit map data
 structure.

These show off the new mocking code by mocking the circuitmux code
so that we can test the circuit map code in isolation.
---
 changes/fancy_testing       |   5 +-
 src/or/circuitlist.c        |   5 +-
 src/or/circuitlist.h        |   6 ++
 src/or/circuitmux.c         |  10 +--
 src/or/circuitmux.h         |   8 +-
 src/test/include.am         |   1 +
 src/test/test.c             |   2 +
 src/test/test_circuitlist.c | 168 ++++++++++++++++++++++++++++++++++++
 8 files changed, 191 insertions(+), 14 deletions(-)
 create mode 100644 src/test/test_circuitlist.c

diff --git a/changes/fancy_testing b/changes/fancy_testing
index 89876ff270..fa5b5703c4 100644
--- a/changes/fancy_testing
+++ b/changes/fancy_testing
@@ -22,7 +22,6 @@
       stub functions, so that the tests can check the function without
       invoking the other functions it calls.
 
-
-
-
+    - Add more unit tests for the <circid,channel>->circuit map, and
+      the destroy-cell-tracking code to fix bug 7912.
 
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 1912b91ddb..70c8980055 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -8,7 +8,7 @@
  * \file circuitlist.c
  * \brief Manage the global circuit list.
  **/
-
+#define CIRCUITLIST_PRIVATE
 #include "or.h"
 #include "channel.h"
 #include "circuitbuild.h"
@@ -41,7 +41,6 @@ circuit_t *global_circuitlist=NULL;
 /** A list of all the circuits in CIRCUIT_STATE_CHAN_WAIT. */
 static smartlist_t *circuits_pending_chans = NULL;
 
-static void circuit_free(circuit_t *circ);
 static void circuit_free_cpath(crypt_path_t *cpath);
 static void circuit_free_cpath_node(crypt_path_t *victim);
 static void cpath_ref_decref(crypt_path_reference_t *cpath_ref);
@@ -736,7 +735,7 @@ or_circuit_new(circid_t p_circ_id, channel_t *p_chan)
 
 /** Deallocate space associated with circ.
  */
-static void
+STATIC void
 circuit_free(circuit_t *circ)
 {
   void *mem;
diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h
index 03f678c5af..a5a54859b2 100644
--- a/src/or/circuitlist.h
+++ b/src/or/circuitlist.h
@@ -12,6 +12,8 @@
 #ifndef TOR_CIRCUITLIST_H
 #define TOR_CIRCUITLIST_H
 
+#include "testsupport.h"
+
 circuit_t * circuit_get_global_list_(void);
 const char *circuit_state_to_string(int state);
 const char *circuit_purpose_to_controller_string(uint8_t purpose);
@@ -68,5 +70,9 @@ void circuits_handle_oom(size_t current_allocation);
 void channel_note_destroy_pending(channel_t *chan, circid_t id);
 void channel_note_destroy_not_pending(channel_t *chan, circid_t id);
 
+#ifdef CIRCUITLIST_PRIVATE
+STATIC void circuit_free(circuit_t *circ);
+#endif
+
 #endif
 
diff --git a/src/or/circuitmux.c b/src/or/circuitmux.c
index c84e0ce09c..081416368e 100644
--- a/src/or/circuitmux.c
+++ b/src/or/circuitmux.c
@@ -922,9 +922,9 @@ circuitmux_num_circuits(circuitmux_t *cmux)
  * Attach a circuit to a circuitmux, for the specified direction.
  */
 
-void
-circuitmux_attach_circuit(circuitmux_t *cmux, circuit_t *circ,
-                          cell_direction_t direction)
+MOCK_IMPL(void,
+circuitmux_attach_circuit,(circuitmux_t *cmux, circuit_t *circ,
+                           cell_direction_t direction))
 {
   channel_t *chan = NULL;
   uint64_t channel_id;
@@ -1071,8 +1071,8 @@ circuitmux_attach_circuit(circuitmux_t *cmux, circuit_t *circ,
  * no-op if not attached.
  */
 
-void
-circuitmux_detach_circuit(circuitmux_t *cmux, circuit_t *circ)
+MOCK_IMPL(void,
+circuitmux_detach_circuit,(circuitmux_t *cmux, circuit_t *circ))
 {
   chanid_circid_muxinfo_t search, *hashent = NULL;
   /*
diff --git a/src/or/circuitmux.h b/src/or/circuitmux.h
index a6bc415cdd..ffa28be06d 100644
--- a/src/or/circuitmux.h
+++ b/src/or/circuitmux.h
@@ -10,6 +10,7 @@
 #define TOR_CIRCUITMUX_H
 
 #include "or.h"
+#include "testsupport.h"
 
 typedef struct circuitmux_policy_s circuitmux_policy_t;
 typedef struct circuitmux_policy_data_s circuitmux_policy_data_t;
@@ -127,9 +128,10 @@ void circuitmux_notify_xmit_cells(circuitmux_t *cmux, circuit_t *circ,
 void circuitmux_notify_xmit_destroy(circuitmux_t *cmux);
 
 /* Circuit interface */
-void circuitmux_attach_circuit(circuitmux_t *cmux, circuit_t *circ,
-                               cell_direction_t direction);
-void circuitmux_detach_circuit(circuitmux_t *cmux, circuit_t *circ);
+MOCK_DECL(void, circuitmux_attach_circuit, (circuitmux_t *cmux, circuit_t *circ,
+                                            cell_direction_t direction));
+MOCK_DECL(void, circuitmux_detach_circuit,
+          (circuitmux_t *cmux, circuit_t *circ));
 void circuitmux_clear_num_cells(circuitmux_t *cmux, circuit_t *circ);
 void circuitmux_set_num_cells(circuitmux_t *cmux, circuit_t *circ,
                               unsigned int n_cells);
diff --git a/src/test/include.am b/src/test/include.am
index 989cf4ebfc..279369e74f 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -19,6 +19,7 @@ src_test_test_SOURCES = \
 	src/test/test.c \
 	src/test/test_addr.c \
 	src/test/test_cell_formats.c \
+	src/test/test_circuitlist.c \
 	src/test/test_containers.c \
 	src/test/test_crypto.c \
 	src/test/test_data.c \
diff --git a/src/test/test.c b/src/test/test.c
index 7721d2c730..d7d4c6c062 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -2132,6 +2132,7 @@ extern struct testcase_t config_tests[];
 extern struct testcase_t introduce_tests[];
 extern struct testcase_t replaycache_tests[];
 extern struct testcase_t cell_format_tests[];
+extern struct testcase_t circuitlist_tests[];
 
 static struct testgroup_t testgroups[] = {
   { "", test_array },
@@ -2147,6 +2148,7 @@ static struct testgroup_t testgroups[] = {
   { "config/", config_tests },
   { "replaycache/", replaycache_tests },
   { "introduce/", introduce_tests },
+  { "circuitlist/", circuitlist_tests },
   END_OF_GROUPS
 };
 
diff --git a/src/test/test_circuitlist.c b/src/test/test_circuitlist.c
new file mode 100644
index 0000000000..718f0cda51
--- /dev/null
+++ b/src/test/test_circuitlist.c
@@ -0,0 +1,168 @@
+/* Copyright (c) 2013, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#define TOR_CHANNEL_INTERNAL_
+#define CIRCUITLIST_PRIVATE
+#include "or.h"
+#include "channel.h"
+#include "circuitlist.h"
+#include "test.h"
+
+static channel_t *
+new_fake_channel(void)
+{
+  channel_t *chan = tor_malloc_zero(sizeof(channel_t));
+  channel_init(chan);
+  return chan;
+}
+
+static struct {
+  int ncalls;
+  void *cmux;
+  void *circ;
+  cell_direction_t dir;
+} cam;
+
+static void
+circuitmux_attach_mock(circuitmux_t *cmux, circuit_t *circ,
+                         cell_direction_t dir)
+{
+  ++cam.ncalls;
+  cam.cmux = cmux;
+  cam.circ = circ;
+  cam.dir = dir;
+}
+
+static struct {
+  int ncalls;
+  void *cmux;
+  void *circ;
+} cdm;
+
+static void
+circuitmux_detach_mock(circuitmux_t *cmux, circuit_t *circ)
+{
+  ++cdm.ncalls;
+  cdm.cmux = cmux;
+  cdm.circ = circ;
+}
+
+#define GOT_CMUX_ATTACH(mux_, circ_, dir_) do {  \
+    tt_int_op(cam.ncalls, ==, 1);                \
+    tt_ptr_op(cam.cmux, ==, (mux_));             \
+    tt_ptr_op(cam.circ, ==, (circ_));            \
+    tt_ptr_op(cam.dir, ==, (dir_));              \
+    memset(&cam, 0, sizeof(cam));                \
+  } while (0)
+
+#define GOT_CMUX_DETACH(mux_, circ_) do {        \
+    tt_int_op(cdm.ncalls, ==, 1);                \
+    tt_ptr_op(cdm.cmux, ==, (mux_));             \
+    tt_ptr_op(cdm.circ, ==, (circ_));            \
+    memset(&cdm, 0, sizeof(cdm));                \
+  } while (0)
+
+
+static void
+test_clist_maps(void *arg)
+{
+  channel_t *ch1 = new_fake_channel();
+  channel_t *ch2 = new_fake_channel();
+  channel_t *ch3 = new_fake_channel();
+  or_circuit_t *or_c1=NULL, *or_c2=NULL;
+
+  MOCK(circuitmux_attach_circuit, circuitmux_attach_mock);
+  MOCK(circuitmux_detach_circuit, circuitmux_detach_mock);
+  memset(&cam, 0, sizeof(cam));
+  memset(&cdm, 0, sizeof(cdm));
+
+  ch1->cmux = (void*)0x1001;
+  ch2->cmux = (void*)0x1002;
+  ch3->cmux = (void*)0x1003;
+
+  or_c1 = or_circuit_new(100, ch2);
+  tt_assert(or_c1);
+  GOT_CMUX_ATTACH(ch2->cmux, or_c1, CELL_DIRECTION_IN);
+  tt_int_op(or_c1->p_circ_id, ==, 100);
+  tt_ptr_op(or_c1->p_chan, ==, ch2);
+
+  or_c2 = or_circuit_new(100, ch1);
+  tt_assert(or_c2);
+  GOT_CMUX_ATTACH(ch1->cmux, or_c2, CELL_DIRECTION_IN);
+  tt_int_op(or_c2->p_circ_id, ==, 100);
+  tt_ptr_op(or_c2->p_chan, ==, ch1);
+
+  circuit_set_n_circid_chan(TO_CIRCUIT(or_c1), 200, ch1);
+  GOT_CMUX_ATTACH(ch1->cmux, or_c1, CELL_DIRECTION_OUT);
+
+  circuit_set_n_circid_chan(TO_CIRCUIT(or_c2), 200, ch2);
+  GOT_CMUX_ATTACH(ch2->cmux, or_c2, CELL_DIRECTION_OUT);
+
+  tt_ptr_op(circuit_get_by_circid_channel(200, ch1), ==, TO_CIRCUIT(or_c1));
+  tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, TO_CIRCUIT(or_c2));
+  tt_ptr_op(circuit_get_by_circid_channel(100, ch2), ==, TO_CIRCUIT(or_c1));
+  /* Try the same thing again, to test the "fast" path. */
+  tt_ptr_op(circuit_get_by_circid_channel(100, ch2), ==, TO_CIRCUIT(or_c1));
+  tt_assert(circuit_id_in_use_on_channel(100, ch2));
+  tt_assert(! circuit_id_in_use_on_channel(101, ch2));
+
+  /* Try changing the circuitid and channel of that circuit. */
+  circuit_set_p_circid_chan(or_c1, 500, ch3);
+  GOT_CMUX_DETACH(ch2->cmux, TO_CIRCUIT(or_c1));
+  GOT_CMUX_ATTACH(ch3->cmux, TO_CIRCUIT(or_c1), CELL_DIRECTION_IN);
+  tt_ptr_op(circuit_get_by_circid_channel(100, ch2), ==, NULL);
+  tt_assert(! circuit_id_in_use_on_channel(100, ch2));
+  tt_ptr_op(circuit_get_by_circid_channel(500, ch3), ==, TO_CIRCUIT(or_c1));
+
+  /* Now let's see about destroy handling. */
+  tt_assert(! circuit_id_in_use_on_channel(205, ch2));
+  tt_assert(circuit_id_in_use_on_channel(200, ch2));
+  channel_note_destroy_pending(ch2, 200);
+  channel_note_destroy_pending(ch2, 205);
+  channel_note_destroy_pending(ch1, 100);
+  tt_assert(circuit_id_in_use_on_channel(205, ch2))
+  tt_assert(circuit_id_in_use_on_channel(200, ch2));
+  tt_assert(circuit_id_in_use_on_channel(100, ch1));
+
+  tt_assert(TO_CIRCUIT(or_c2)->n_delete_pending != 0);
+  tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, TO_CIRCUIT(or_c2));
+  tt_ptr_op(circuit_get_by_circid_channel(100, ch1), ==, TO_CIRCUIT(or_c2));
+
+  /* Okay, now free ch2 and make sure that the circuit ID is STILL not
+   * usable, because we haven't declared the destroy to be nonpending */
+  tt_int_op(cdm.ncalls, ==, 0);
+  circuit_free(TO_CIRCUIT(or_c2));
+  or_c2 = NULL; /* prevent free */
+  tt_int_op(cdm.ncalls, ==, 2);
+  memset(&cdm, 0, sizeof(cdm));
+  tt_assert(circuit_id_in_use_on_channel(200, ch2));
+  tt_assert(circuit_id_in_use_on_channel(100, ch1));
+  tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, NULL);
+  tt_ptr_op(circuit_get_by_circid_channel(100, ch1), ==, NULL);
+
+  /* Now say that the destroy is nonpending */
+  channel_note_destroy_not_pending(ch2, 200);
+  tt_ptr_op(circuit_get_by_circid_channel(200, ch2), ==, NULL);
+  channel_note_destroy_not_pending(ch1, 100);
+  tt_ptr_op(circuit_get_by_circid_channel(100, ch1), ==, NULL);
+  tt_assert(! circuit_id_in_use_on_channel(200, ch2));
+  tt_assert(! circuit_id_in_use_on_channel(100, ch1));
+
+
+done:
+  tor_free(ch1);
+  tor_free(ch2);
+  tor_free(ch3);
+  if (or_c1)
+    circuit_free(TO_CIRCUIT(or_c1));
+  if (or_c2)
+    circuit_free(TO_CIRCUIT(or_c2));
+  UNMOCK(circuitmux_attach_circuit);
+  UNMOCK(circuitmux_detach_circuit);
+}
+
+
+struct testcase_t circuitlist_tests[] = {
+  { "maps", test_clist_maps, TT_FORK, NULL, NULL },
+  END_OF_TESTCASES
+};