From 5bd86b50b58ffcad48416df6fa6b411e92c4636e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 08:32:56 -0500 Subject: [PATCH 01/28] codetool: post-processor for clang-format This code transformer makes a couple of changes that we want for our source code, and can be expanded to handle more. --- scripts/maint/codetool.py | 174 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100755 scripts/maint/codetool.py diff --git a/scripts/maint/codetool.py b/scripts/maint/codetool.py new file mode 100755 index 0000000000..6336e6843b --- /dev/null +++ b/scripts/maint/codetool.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020, The Tor Project, Inc. +# See LICENSE for licensing information. + +""" + This program uses a set of plugable filters to inspect and transform + our C code. +""" + +import os +import re +import sys + +class Filter: + """A Filter transforms a string containing a C program.""" + def __init__(self): + pass + + def transform(self, s): + return s + +class CompoundFilt(Filter): + """A CompoundFilt runs another set of filters, in sequence.""" + def __init__(self, items=()): + super().__init__() + self._filters = list(items) + + def add(self, filt): + self._filters.append(filt) + return self + + def transform(self, s): + for f in self._filters: + s = f.transform(s) + + return s + +class SplitError(Exception): + """Exception: raised if split_comments() can't understand a C file.""" + pass + +def split_comments(s): + r"""Iterate over the C code in 's', and yield a sequence of (code, + comment) pairs. Each pair will contain either a nonempty piece + of code, a nonempty comment, or both. + + >>> list(split_comments("hello // world\n")) + [('hello ', '// world'), ('\n', '')] + + >>> list(split_comments("a /* b cd */ efg // hi")) + [('a ', '/* b cd */'), (' efg ', '// hi')] + """ + + # Matches a block of code without any comments. + PAT_CODE = re.compile(r'''^(?: [^/"']+ | + "(?:[^\\"]+|\\.)*" | + '(?:[^\\']+|\\.)*' | + /[^/*] + )*''', re.VERBOSE|re.DOTALL) + + # Matches a C99 "//" comment. + PAT_C99_COMMENT = re.compile(r'^//.*$', re.MULTILINE) + + # Matches a C "/* */" comment. + PAT_C_COMMENT = re.compile(r'^/\*(?:[^*]|\*+[^*/])*\*+/', re.DOTALL) + + while True: + # Find some non-comment code at the start of the string. + m = PAT_CODE.match(s) + + # If we found some code here, save it and advance the string. + # Otherwise set 'code' to "". + if m: + code = m.group(0) + s = s[m.end():] + else: + code = "" + + # Now we have a comment, or the end of the string. Find out which + # one, and how long it is. + if s.startswith("//"): + m = PAT_C99_COMMENT.match(s) + else: + m = PAT_C_COMMENT.match(s) + + # If we got a comment, save it and advance the string. Otherwise + # set 'comment' to "". + if m: + comment = m.group(0) + s = s[m.end():] + else: + comment = "" + + # If we found no code and no comment, we should be at the end of + # the string... + if code == "" and comment == "": + if s: + # But in case we *aren't* at the end of the string, raise + # an error. + raise SplitError() + # ... all is well, we're done scanning the code. + return + + yield (code, comment) + +class IgnoreCommentsFilt(Filter): + """Wrapper: applies another filter to C code only, excluding comments. + """ + def __init__(self, filt): + super().__init__() + self._filt = filt + + def transform(self, s): + result = [] + for code, comment in split_comments(s): + result.append(self._filt.transform(code)) + result.append(comment) + return "".join(result) + + +class RegexFilt(Filter): + """A regex filter applies a regular expression to some C code.""" + def __init__(self, pat, replacement, flags=0): + super().__init__() + self._pat = re.compile(pat, flags) + self._replacement = replacement + + def transform(self, s): + s, _ = self._pat.subn(self._replacement, s) + return s + +def revise(fname, filt): + """Run 'filt' on the contents of the file in 'fname'. If any + changes are made, then replace the file with its new contents. + Otherwise, leave the file alone. + """ + contents = open(fname, 'r').read() + result = filt.transform(contents) + if result == contents: + return + + tmpname = "{}_codetool_tmp".format(fname) + try: + with open(tmpname, 'w') as f: + f.write(result) + os.rename(tmpname, fname) + except: + os.unlink(tmpname) + raise + +############################## +# Filtering rules. +############################## + +# Make sure that there is a newline after the first comma in a MOCK_IMPL() +BREAK_MOCK_IMPL = RegexFilt( + r'^MOCK_IMPL\(([^,]+),\s*(\S+)', + r'MOCK_IMPL(\1,\n\2', + re.MULTILINE) + +# Make sure there is no newline between } and a loop iteration terminator. +RESTORE_SMARTLIST_END = RegexFilt( + r'}\s*(SMARTLIST|DIGESTMAP|DIGEST256MAP|STRMAP|MAP)_FOREACH_END\s*\(', + r'} \1_FOREACH_END (', + re.MULTILINE) + +F = CompoundFilt() +F.add(IgnoreCommentsFilt(CompoundFilt([ + RESTORE_SMARTLIST_END, + BREAK_MOCK_IMPL]))) + +if __name__ == '__main__': + for fname in sys.argv[1:]: + revise(fname, F) From 573ab70bfe4c6239a3fffda89b1a95e4e7ca10aa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 16 Dec 2019 10:58:32 -0500 Subject: [PATCH 02/28] Temporary clang-format configuration and script. The format is the same as in my previous efforts here. The script is a little tricky, since it invokes both clang-format and codetool, and it makes sure that files do not have a changed mtime unless there is actually some change in the file. --- .clang-format | 166 ++++++++++++++++++++++++++++++++++ scripts/maint/clang-format.sh | 34 +++++++ 2 files changed, 200 insertions(+) create mode 100644 .clang-format create mode 100755 scripts/maint/clang-format.sh diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000000..31b76a0025 --- /dev/null +++ b/.clang-format @@ -0,0 +1,166 @@ +--- +Language: Cpp +# Out of all supported styles, LLVM seems closest to our own. +BasedOnStyle: LLVM + +################ +# +# Deviations from LLVM's style. +# +################ + +# We prefer an indentation width of 4 columns; LLVM likes 2. +## OVERRIDE FOR COMPARISON +IndentWidth: 2 + +## OVERRIDE FOR COMPARISON +## for now i'm not sorting includes, since that makes every file get touched. +SortIncludes: false + +# We prefer 79; llvm likes 80. +ColumnLimit: 79 + +# Where do we want to put backslashes on multiline macros? Our choices are +# "as far left as possible", "as far right as possible", and "make no changes." +# LLVM defaults to right, but we don't dig that. +AlignEscapedNewlines: Left + +# When we see a bunch of things in a row with comments after them, should we +# try to align those comments? Doing so makes some of our code pretty ugly. +AlignTrailingComments: false + +# We use a function declaration style much closer to BSD KNF than to LLVM's. +# We say: +# int foo(int x); +# int +# foo(int x) +# { +# ... +# } +# whereas llvm prefers: +# int foo(int x); +# int foo(int x) { +# ... +# } +# or even: +# int foo(int x) { ... } +# +BreakBeforeBraces: Custom +BraceWrapping: + AfterFunction: true +AllowShortFunctionsOnASingleLine: None +AlwaysBreakAfterReturnType: AllDefinitions + +# We don't like blocks to start with an empty line. +# +KeepEmptyLinesAtTheStartOfBlocks: false + +################ +# +# Tor-specific magic +# +################ + +# +# These comments are magical, and should not be changed. +# +CommentPragmas: 'LCOV_EXCL|COVERITY' + +# +# Remove duplicate empty lines. +# +MaxEmptyLinesToKeep: 1 + +# +# Indent preprocessor directives, for clarity. +# +IndentPPDirectives: AfterHash + +# +# These introduce an iteration, and work a bit like a for loop. +# +# Note that we can NOT include ones that don't work like "for". For example, +# if the body is an argument to the macro, we can't list it here. +# +ForEachMacros: + - MAP_FOREACH + - MAP_FOREACH_MODIFY + - TOR_SIMPLEQ_FOREACH + - TOR_SIMPLEQ_FOREACH_SAFE + - TOR_SLIST_FOREACH + - TOR_SLIST_FOREACH_SAFE + - TOR_LIST_FOREACH + - TOR_LIST_FOREACH_SAFE + - TOR_TAILQ_FOREACH + - TOR_TAILQ_FOREACH_SAFE + - TOR_TAILQ_FOREACH_REVERSE + - TOR_TAILQ_FOREACH_REVERSE_SAFE + - TOR_CIRCLEQ_FOREACH + - TOR_CIRCLEQ_FOREACH_SAFE + - TOR_CIRCLEQ_FOREACH_REVERSE + - TOR_CIRCLEQ_FOREACH_REVERSE_SAFE + - HT_FOREACH + - SMARTLIST_FOREACH_BEGIN + - DIGESTMAP_FOREACH + - DIGESTMAP_FOREACH_MODIFY + - DIGEST256MAP_FOREACH + - DIGEST256MAP_FOREACH_MODIFY + - SDMAP_FOREACH + - RIMAP_FOREACH + - EIMAP_FOREACH + +# +# Omitting: +# +# - SMARTLIST_FOREACH, since the body of the loop is an argument. + +# +# This explains how to sort our headers. +# +# This is more complex than it truly should be, but I've edited this till +# compilation still mostly passes. +# +# I'm disabling this, however, since it's a distraction from the other +# formatting issues. See SortIncludes above. +# +IncludeCategories: + - Regex: '^"orconfig.h' + Priority: -30 + - Regex: '^"ext/' + Priority: -18 + - Regex: '^"lib/' + Priority: -10 + - Regex: '^"core/or/or.h' + Priority: -5 + - Regex: '^"core/' + Priority: 5 + - Regex: '^"feature/' + Priority: 10 + - Regex: '^"app/' + Priority: 20 + +# +# These macros should always cause indentation, as though they were { and }. +# +# Do NOT put macros here unless you want an extra level of indentation between +# them whenever they appear. +# +MacroBlockBegin: "^STMT_BEGIN|TT_STMT_BEGIN$" +MacroBlockEnd: "^STMT_END|TT_STMT_END$" + +# +# These macros don't need to have semicolons afterwards. +# +StatementMacros: + - HT_PROTOTYPE + - HT_GENERATE + - HT_GENERATE2 + +# +# These macros are interpreted as types. +# (Not supported in my clang-format) +# +# TypenameMacros: +# - "STACK_OF" + +... diff --git a/scripts/maint/clang-format.sh b/scripts/maint/clang-format.sh new file mode 100755 index 0000000000..86430b9b26 --- /dev/null +++ b/scripts/maint/clang-format.sh @@ -0,0 +1,34 @@ +#!/bin/sh +# Copyright 2020, The Tor Project, Inc. +# See LICENSE for licensing information. + +# This script runs "clang-format" and "codetool" in sequence over each of +# our source files, and replaces the original file only if it has changed. +# +# We can't just use clang-format -i, since we also want to use codetool to +# reformat a few things back to how we want them, and we want avoid changing +# the mtime on files that didn't actually change. + +set -e + +cd "$(dirname "$0")/../../src/" + +# Shellcheck complains that a for loop over find's output is unreliable, +# since there might be special characters in the output. But we happen +# to know that none of our C files have special characters or spaces in +# their names, so this is safe. +# +# shellcheck disable=SC2044 +for fname in $(find lib core feature app test tools -name '[^.]*.[ch]'); do + tmpfname="${fname}.clang_fmt.tmp" + rm -f "${tmpfname}" + clang-format --style=file "${fname}" > "${tmpfname}" + ../scripts/maint/codetool.py "${tmpfname}" + if cmp "${fname}" "${tmpfname}" >/dev/null 2>&1; then + echo "No change in ${fname}" + rm -f "${tmpfname}" + else + echo "Change in ${fname}" + mv "${tmpfname}" "${fname}" + fi +done From f1d371bf32d33cee95df1b13daa43686f81d94ef Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 Dec 2019 13:58:01 -0500 Subject: [PATCH 03/28] checkSpace: remove the nosplabel test. --- scripts/maint/checkSpace.pl | 6 +++--- scripts/maint/checkspace_tests/expected.txt | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index f4e6f733c8..96a256968a 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -58,9 +58,9 @@ for my $fn (@ARGV) { } ## Warn about labels that don't have a space in front of them # (We indent every label at least one space) - if (/^[a-zA-Z_][a-zA-Z_0-9]*:/) { - msg "nosplabel:$fn:$.\n"; - } + #if (/^[a-zA-Z_][a-zA-Z_0-9]*:/) { + # msg "nosplabel:$fn:$.\n"; + #} ## Warn about trailing whitespace. # (We don't allow whitespace at the end of the line; make your # editor highlight it for you so you can stop adding it in.) diff --git a/scripts/maint/checkspace_tests/expected.txt b/scripts/maint/checkspace_tests/expected.txt index 935b750ef9..38595ed373 100644 --- a/scripts/maint/checkspace_tests/expected.txt +++ b/scripts/maint/checkspace_tests/expected.txt @@ -5,7 +5,6 @@ tp fn():./dubious.c:15 Wide:./dubious.c:17 TAB:./dubious.c:24 - nosplabel:./dubious.c:26 CR:./dubious.c:30 Space@EOL:./dubious.c:32 non-K&R {:./dubious.c:39 From 15819cde6163e43ac86d1be078a7b6b4e3ed7a02 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 9 Jan 2020 16:21:17 -0500 Subject: [PATCH 04/28] checkSpace.pl: allow {{, ){, and ({. --- scripts/maint/checkSpace.pl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index 96a256968a..af81db8d53 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -156,9 +156,8 @@ for my $fn (@ARGV) { # msg "//:$fn:$.\n"; s!//.*!!; } - ## Warn about unquoted braces preceded by non-space. - # (No character except a space should come before a {) - if (/([^\s'])\{/) { + ## Warn about unquoted braces preceded by unexpected character. + if (/([^\s'\)\(\{])\{/) { msg "$1\{:$fn:$.\n"; } ## Warn about double semi-colons at the end of a line. From 1f1d943999bd0ade5153074bb90ee9a40440825f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 08:04:28 -0500 Subject: [PATCH 05/28] checkspace: allow spaces in cpp directives. --- scripts/maint/checkSpace.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index af81db8d53..3080fafd3e 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -128,12 +128,12 @@ for my $fn (@ARGV) { if ($isheader) { if ($seenguard == 0) { - if (/ifndef\s+(\S+)/) { + if (/^\s*\#\s*ifndef\s+(\S+)/) { ++$seenguard; $guardname = $1; } } elsif ($seenguard == 1) { - if (/^\#define (\S+)/) { + if (/^\s*\#\s*define (\S+)/) { ++$seenguard; if ($1 ne $guardname) { msg "GUARD:$fn:$.: Header guard macro mismatch.\n"; From bfa760738521715f8c858de7584fd08cd90cb538 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 08:31:35 -0500 Subject: [PATCH 06/28] checkSpace.pl: Use a data structure for a list of non-function names --- scripts/maint/checkSpace.pl | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index 3080fafd3e..cc77d8d5be 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -23,6 +23,12 @@ if ($ARGV[0] =~ /^-/) { $C = ($lang eq '-C'); } +# hashmap of things where we allow spaces between them and (. +our %allow_space_after= map {$_, 1} qw{ + if while for switch return int unsigned elsif WINAPI + void __attribute__ op size_t double uint64_t workqueue_reply_t bool +}; + our %basenames = (); our %guardnames = (); @@ -177,12 +183,7 @@ for my $fn (@ARGV) { # (Don't put a space between the name of a function and its # arguments.) if (/(\w+)\s\(([A-Z]*)/) { - if ($1 ne "if" and $1 ne "while" and $1 ne "for" and - $1 ne "switch" and $1 ne "return" and $1 ne "int" and - $1 ne "elsif" and $1 ne "WINAPI" and $2 ne "WINAPI" and - $1 ne "void" and $1 ne "__attribute__" and $1 ne "op" and - $1 ne "size_t" and $1 ne "double" and $1 ne "uint64_t" and - $1 ne "workqueue_reply_t" and $1 ne "bool") { + if (! $allow_space_after{$1} && $2 ne 'WINAPI') { msg "fn ():$fn:$.\n"; } } From 8d6f27cea58627e08e7872c785456235122e060b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 08:32:15 -0500 Subject: [PATCH 07/28] checkSpace.pl: Allow space between iteration macros and (). Clang-format wants to put these in, and they do make sense for consistency. Also allow more types. --- scripts/maint/checkSpace.pl | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index cc77d8d5be..a5480d5ba4 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -26,7 +26,20 @@ if ($ARGV[0] =~ /^-/) { # hashmap of things where we allow spaces between them and (. our %allow_space_after= map {$_, 1} qw{ if while for switch return int unsigned elsif WINAPI - void __attribute__ op size_t double uint64_t workqueue_reply_t bool + void __attribute__ op size_t double uint64_t + bool ssize_t + workqueue_reply_t hs_desc_decode_status_t + PRStatus + SMARTLIST_FOREACH_BEGIN SMARTLIST_FOREACH_END + HT_FOREACH + DIGESTMAP_FOREACH_MODIFY DIGESTMAP_FOREACH + DIGEST256MAP_FOREACH_MODIFY DIGEST256MAP_FOREACH + STRMAP_FOREACH_MODIFY STRMAP_FOREACH + SDMAP_FOREACH EIMAP_FOREACH RIMAP_FOREACH + MAP_FOREACH_MODIFY MAP_FOREACH + TOR_SIMPLEQ_FOREACH TOR_SIMPLEQ_FOREACH_SAFE + TOR_LIST_FOREACH TOR_LIST_FOREACH_SAFE + TOR_SLIST_FOREACH TOR_SLIST_FOREACH_SAFE }; our %basenames = (); From 98fdc3e41a79625a5f4b51689a7c76fed4228d00 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 08:55:18 -0500 Subject: [PATCH 08/28] Use a compile-time assert in control_events.h (The original idiom here led clang-format to generating a too-wide line.) --- src/feature/control/control_events.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/feature/control/control_events.h b/src/feature/control/control_events.h index 74bbc0047d..535c8e5887 100644 --- a/src/feature/control/control_events.h +++ b/src/feature/control/control_events.h @@ -12,6 +12,7 @@ #ifndef TOR_CONTROL_EVENTS_H #define TOR_CONTROL_EVENTS_H +#include "lib/cc/ctassert.h" #include "core/or/ocirc_event.h" #include "core/or/orconn_event.h" @@ -287,10 +288,7 @@ typedef uint64_t event_mask_t; /* If EVENT_MAX_ ever hits 0x0040, we need to make the mask into a * different structure, as it can only handle a maximum left shift of 1<<63. */ - -#if EVENT_MAX_ >= EVENT_CAPACITY_ -#error control_connection_t.event_mask has an event greater than its capacity -#endif +CTASSERT(EVENT_MAX_ < EVENT_CAPACITY_); #define EVENT_MASK_(e) (((uint64_t)1)<<(e)) From 9feeb4cf97d98c7b8cf3c7e871b239cea835f3f4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 08:58:39 -0500 Subject: [PATCH 09/28] prob_distr: use "clang-format off" to avoid wide lines for URLs --- src/lib/math/prob_distr.c | 8 +++++--- src/test/test_prob_distr.c | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lib/math/prob_distr.c b/src/lib/math/prob_distr.c index 548d256023..31d485120e 100644 --- a/src/lib/math/prob_distr.c +++ b/src/lib/math/prob_distr.c @@ -1284,15 +1284,16 @@ sample_genpareto_locscale(uint32_t s, double p0, double mu, double sigma, /** * Deterministically sample from the geometric distribution with * per-trial success probability p. - * + **/ +// clang-format off +/* * XXX Quantify the error (KL divergence?) of this * ceiling-of-exponential sampler from a true geometric distribution, * which we could get by rejection sampling. Relevant papers: * * John F. Monahan, `Accuracy in Random Number Generation', * Mathematics of Computation 45(172), October 1984, pp. 559--568. -*https://pdfs.semanticscholar.org/aca6/74b96da1df77b2224e8cfc5dd6d61a471632.pdf - * +https://pdfs.semanticscholar.org/aca6/74b96da1df77b2224e8cfc5dd6d61a471632.pdf * Karl Bringmann and Tobias Friedrich, `Exact and Efficient * Generation of Geometric Random Variates and Random Graphs', in * Proceedings of the 40th International Colloaquium on Automata, @@ -1301,6 +1302,7 @@ sample_genpareto_locscale(uint32_t s, double p0, double mu, double sigma, * https://doi.org/10.1007/978-3-642-39206-1_23 * https://people.mpi-inf.mpg.de/~kbringma/paper/2013ICALP-1.pdf */ +// clang-format on static double sample_geometric(uint32_t s, double p0, double p) { diff --git a/src/test/test_prob_distr.c b/src/test/test_prob_distr.c index c3d1c80d70..c5423ce14a 100644 --- a/src/test/test_prob_distr.c +++ b/src/test/test_prob_distr.c @@ -1223,14 +1223,16 @@ test_stochastic_weibull_impl(double lambda, double k) .k = k, }; +// clang-format off /* * XXX Consider applying a Tiku-Singh test: * * M.L. Tiku and M. Singh, `Testing the two-parameter * Weibull distribution', Communications in Statistics -- * Theory and Methods A10(9), 1981, 907--918. - *https://www.tandfonline.com/doi/pdf/10.1080/03610928108828082?needAccess=true +https://www.tandfonline.com/doi/pdf/10.1080/03610928108828082?needAccess=true */ +// clang-format on return test_psi_dist_sample(&dist.base); } From c8fae6b5c8c76089da37c169defbc63a53300a3f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 09:25:04 -0500 Subject: [PATCH 10/28] checkSpace: don't treat an unindented label as starting a function. --- scripts/maint/checkSpace.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index a5480d5ba4..bf1c69e00a 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -207,7 +207,7 @@ for my $fn (@ARGV) { if ($in_func_head || ($fn !~ /\.h$/ && /^[a-zA-Z0-9_]/ && ! /^(?:const |static )*(?:typedef|struct|union)[^\(]*$/ && - ! /= *\{$/ && ! /;$/)) { + ! /= *\{$/ && ! /;$/) && ! /^[a-zA-Z0-9_]+\s*:/) { if (/.\{$/){ msg "fn() {:$fn:$.\n"; $in_func_head = 0; From e9b663beaf153359033d78934cdf8f3d8bb9f2d9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 09:25:40 -0500 Subject: [PATCH 11/28] onion_queue.c: use TAILQ_HEAD less confusingly. When we use macro inline, clang-format likes to break it in the middle, which makes checkSpace get confused. --- src/feature/relay/onion_queue.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/feature/relay/onion_queue.c b/src/feature/relay/onion_queue.c index ce2d41b7e1..3cbaa65d28 100644 --- a/src/feature/relay/onion_queue.c +++ b/src/feature/relay/onion_queue.c @@ -49,10 +49,12 @@ typedef struct onion_queue_t { /** 5 seconds on the onion queue til we just send back a destroy */ #define ONIONQUEUE_WAIT_CUTOFF 5 +TOR_TAILQ_HEAD(onion_queue_head_t, onion_queue_t); +typedef struct onion_queue_head_t onion_queue_head_t; + /** Array of queues of circuits waiting for CPU workers. An element is NULL * if that queue is empty.*/ -static TOR_TAILQ_HEAD(onion_queue_head_t, onion_queue_t) - ol_list[MAX_ONION_HANDSHAKE_TYPE+1] = +static onion_queue_head_t ol_list[MAX_ONION_HANDSHAKE_TYPE+1] = { TOR_TAILQ_HEAD_INITIALIZER(ol_list[0]), /* tap */ TOR_TAILQ_HEAD_INITIALIZER(ol_list[1]), /* fast */ TOR_TAILQ_HEAD_INITIALIZER(ol_list[2]), /* ntor */ From 60f01da78eaab9d036ac4af0d6bea27eb210c7cb Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 10:36:51 -0500 Subject: [PATCH 12/28] Use smaller CPP error messages Clang-format wants to split these messages across multiple lines, which confuses the heck out of coccinelle. --- src/feature/dirclient/dirclient.c | 5 ++--- src/lib/cc/compat_compiler.h | 4 ++-- src/lib/cc/torint.h | 7 +++---- src/lib/ctime/di_ops.c | 4 ++-- src/lib/memarea/memarea.c | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/feature/dirclient/dirclient.c b/src/feature/dirclient/dirclient.c index 1b6eed12f0..d2df6f30da 100644 --- a/src/feature/dirclient/dirclient.c +++ b/src/feature/dirclient/dirclient.c @@ -51,6 +51,7 @@ #include "feature/rend/rendservice.h" #include "feature/stats/predict_ports.h" +#include "lib/cc/ctassert.h" #include "lib/compress/compress.h" #include "lib/crypt_ops/crypto_format.h" #include "lib/crypt_ops/crypto_util.h" @@ -1443,9 +1444,7 @@ compare_strs_(const void **a, const void **b) } #define CONDITIONAL_CONSENSUS_FPR_LEN 3 -#if (CONDITIONAL_CONSENSUS_FPR_LEN > DIGEST_LEN) -#error "conditional consensus fingerprint length is larger than digest length" -#endif +CTASSERT(CONDITIONAL_CONSENSUS_FPR_LEN <= DIGEST_LEN); /** Return the URL we should use for a consensus download. * diff --git a/src/lib/cc/compat_compiler.h b/src/lib/cc/compat_compiler.h index 907622f942..571086406c 100644 --- a/src/lib/cc/compat_compiler.h +++ b/src/lib/cc/compat_compiler.h @@ -25,11 +25,11 @@ #endif /* defined(__has_feature) */ #ifndef NULL_REP_IS_ZERO_BYTES -#error "It seems your platform does not represent NULL as zero. We can't cope." +#error "Your platform does not represent NULL as zero. We can't cope." #endif #ifndef DOUBLE_0_REP_IS_ZERO_BYTES -#error "It seems your platform does not represent 0.0 as zeros. We can't cope." +#error "Your platform does not represent 0.0 as zeros. We can't cope." #endif #if 'a'!=97 || 'z'!=122 || 'A'!=65 || ' '!=32 diff --git a/src/lib/cc/torint.h b/src/lib/cc/torint.h index cef1482bdc..af7a90431c 100644 --- a/src/lib/cc/torint.h +++ b/src/lib/cc/torint.h @@ -49,7 +49,7 @@ typedef int32_t ssize_t; * aren't 2's complement, and you don't define LONG_MAX, then you're so * bizarre that I want nothing to do with you. */ #ifndef USING_TWOS_COMPLEMENT -#error "Seems that your platform doesn't use 2's complement arithmetic. Argh." +#error "Your platform doesn't use 2's complement arithmetic." #endif #ifndef TIME_MAX @@ -126,12 +126,11 @@ typedef int32_t ssize_t; #define SIZE_T_CEILING ((size_t)(SSIZE_MAX-16)) #if SIZEOF_INT > SIZEOF_VOID_P -#error "sizeof(int) > sizeof(void *) - Tor cannot be built on this platform!" +#error "sizeof(int) > sizeof(void *) - Can't build Tor here." #endif #if SIZEOF_UNSIGNED_INT > SIZEOF_VOID_P -#error "sizeof(unsigned int) > sizeof(void *) - Tor cannot be built on this \ -platform!" +#error "sizeof(unsigned int) > sizeof(void *) - Can't build Tor here." #endif #endif /* !defined(TOR_TORINT_H) */ diff --git a/src/lib/ctime/di_ops.c b/src/lib/ctime/di_ops.c index 7448a9973e..c14029b877 100644 --- a/src/lib/ctime/di_ops.c +++ b/src/lib/ctime/di_ops.c @@ -72,10 +72,10 @@ tor_memcmp(const void *a, const void *b, size_t len) * actually implementation-defined in standard C. So how do we * get away with assuming it? Easy. We check.) */ #if ((-60 >> 8) != -1) -#error "According to cpp, right-shift doesn't perform sign-extension." +#error "cpp says right-shift doesn't perform sign-extension." #endif #ifndef RSHIFT_DOES_SIGN_EXTEND -#error "According to configure, right-shift doesn't perform sign-extension." +#error "configure says right-shift doesn't perform sign-extension." #endif /* If v1 == v2, equal_p is ~0, so this will leave retval diff --git a/src/lib/memarea/memarea.c b/src/lib/memarea/memarea.c index d677c364a4..4d26c20eeb 100644 --- a/src/lib/memarea/memarea.c +++ b/src/lib/memarea/memarea.c @@ -39,7 +39,7 @@ #elif MEMAREA_ALIGN == 8 #define MEMAREA_ALIGN_MASK ((uintptr_t)7) #else -#error "void* is neither 4 nor 8 bytes long. I don't know how to align stuff." +#error "void* is neither 4 nor 8 bytes long." #endif /* MEMAREA_ALIGN == 4 || ... */ #if defined(__GNUC__) && defined(FLEXIBLE_ARRAY_MEMBER) From efa5020a9c34883543108f7301cb2177526ccaf8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 10:54:35 -0500 Subject: [PATCH 13/28] log_test_helpers: remove semicolons from end of macros We want our code to require semicolons after use of these macros, so that our code formatters and/or analysis tools don't get confused. --- src/test/log_test_helpers.h | 16 ++++++++-------- src/test/test_consdiff.c | 14 +++++++------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/test/log_test_helpers.h b/src/test/log_test_helpers.h index e2ddf09466..c2d71c6bcd 100644 --- a/src/test/log_test_helpers.h +++ b/src/test/log_test_helpers.h @@ -78,7 +78,7 @@ void mock_dump_saved_logs(void); mock_saved_log_n_entries() == 1, \ ("expected log to contain exactly 1 message \"%s\"", \ str)); \ - } while (0); + } while (0) #define expect_single_log_msg_containing(str) \ do { \ @@ -86,30 +86,30 @@ void mock_dump_saved_logs(void); mock_saved_log_n_entries() == 1 , \ ("expected log to contain 1 message, containing \"%s\"",\ str)); \ - } while (0); + } while (0) #define expect_no_log_msg(str) \ assert_log_predicate(!mock_saved_log_has_message(str), \ - ("expected log to not contain \"%s\"",str)); + ("expected log to not contain \"%s\"",str)) #define expect_no_log_msg_containing(str) \ assert_log_predicate(!mock_saved_log_has_message_containing(str), \ - ("expected log to not contain \"%s\"", str)); + ("expected log to not contain \"%s\"", str)) #define expect_log_severity(severity) \ assert_log_predicate(mock_saved_log_has_severity(severity), \ - ("expected log to contain severity " # severity)); + ("expected log to contain severity " # severity)) #define expect_no_log_severity(severity) \ assert_log_predicate(!mock_saved_log_has_severity(severity), \ - ("expected log to not contain severity " # severity)); + ("expected log to not contain severity " # severity)) #define expect_log_entry() \ assert_log_predicate(mock_saved_log_has_entry(), \ - ("expected log to contain entries")); + ("expected log to contain entries")) #define expect_no_log_entry() \ assert_log_predicate(!mock_saved_log_has_entry(), \ - ("expected log to not contain entries")); + ("expected log to not contain entries")) #endif /* !defined(TOR_LOG_TEST_HELPERS_H) */ diff --git a/src/test/test_consdiff.c b/src/test/test_consdiff.c index e4cfece9c3..242e2f7818 100644 --- a/src/test/test_consdiff.c +++ b/src/test/test_consdiff.c @@ -1030,7 +1030,7 @@ test_consdiff_apply_diff(void *arg) /* diff doesn't have enough lines. */ cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); - expect_single_log_msg_containing("too short") + expect_single_log_msg_containing("too short"); /* first line doesn't match format-version string. */ smartlist_add_linecpy(diff, area, "foo-bar"); @@ -1038,7 +1038,7 @@ test_consdiff_apply_diff(void *arg) mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); - expect_single_log_msg_containing("format is not known") + expect_single_log_msg_containing("format is not known"); /* The first word of the second header line is not "hash". */ smartlist_clear(diff); @@ -1048,7 +1048,7 @@ test_consdiff_apply_diff(void *arg) mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); - expect_single_log_msg_containing("does not include the necessary digests") + expect_single_log_msg_containing("does not include the necessary digests"); /* Wrong number of words after "hash". */ smartlist_clear(diff); @@ -1057,7 +1057,7 @@ test_consdiff_apply_diff(void *arg) mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); - expect_single_log_msg_containing("does not include the necessary digests") + expect_single_log_msg_containing("does not include the necessary digests"); /* base16 digests do not have the expected length. */ smartlist_clear(diff); @@ -1067,7 +1067,7 @@ test_consdiff_apply_diff(void *arg) cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); expect_single_log_msg_containing("includes base16-encoded digests of " - "incorrect size") + "incorrect size"); /* base16 digests contain non-base16 characters. */ smartlist_clear(diff); @@ -1078,7 +1078,7 @@ test_consdiff_apply_diff(void *arg) mock_clean_saved_logs(); cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); - expect_single_log_msg_containing("includes malformed digests") + expect_single_log_msg_containing("includes malformed digests"); /* Invalid ed diff. * As tested in apply_ed_diff, but check that apply_diff does return NULL if @@ -1095,7 +1095,7 @@ test_consdiff_apply_diff(void *arg) cons2 = consdiff_apply_diff(cons1, diff, &digests1); tt_ptr_op(NULL, OP_EQ, cons2); expect_single_log_msg_containing("because an ed command was missing a line " - "number") + "number"); /* Base consensus doesn't match its digest as found in the diff. */ smartlist_clear(diff); From 87b71a692a568453b3b4a19cf798d269a9b2fbf9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 11:32:34 -0500 Subject: [PATCH 14/28] Remove senseless CHECK_PRINTF()s from util_bug.c These belong in util_bug.h (and they already are there). Their presence made clang-format misindent these functions in a way that checkSpace.pl dislikes. --- src/lib/log/util_bug.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/log/util_bug.c b/src/lib/log/util_bug.c index de44d30e4e..83045ebb90 100644 --- a/src/lib/log/util_bug.c +++ b/src/lib/log/util_bug.c @@ -71,7 +71,6 @@ tor_set_failed_assertion_callback(void (*fn)(void)) /** Helper for tor_assert: report the assertion failure. */ void -CHECK_PRINTF(5, 6) tor_assertion_failed_(const char *fname, unsigned int line, const char *func, const char *expr, const char *fmt, ...) @@ -104,7 +103,6 @@ tor_assertion_failed_(const char *fname, unsigned int line, /** Helper for tor_assert_nonfatal: report the assertion failure. */ void -CHECK_PRINTF(6, 7) tor_bug_occurred_(const char *fname, unsigned int line, const char *func, const char *expr, int once, const char *fmt, ...) From 6104c407e077175087b7ae117c0ace5ce3897bcf Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 14:07:11 -0500 Subject: [PATCH 15/28] maps: do not include _ as part of expected prefix in macros. Doing this makes our macro usage a little clear IMO, and also avoids having to use an unadorned "new" within a macro. (Clang-format seems to think that "new" means we're doing C++, and so it generates some output that checkSpace.pl doesn't care for.) --- src/feature/nodelist/authcert.c | 2 +- src/feature/nodelist/routerlist.c | 6 +- src/lib/container/map.h | 112 +++++++++++++++--------------- 3 files changed, 60 insertions(+), 60 deletions(-) diff --git a/src/feature/nodelist/authcert.c b/src/feature/nodelist/authcert.c index 7bdfabaeab..9c7525b6e2 100644 --- a/src/feature/nodelist/authcert.c +++ b/src/feature/nodelist/authcert.c @@ -46,7 +46,7 @@ #include "feature/nodelist/networkstatus_voter_info_st.h" #include "feature/nodelist/node_st.h" -DECLARE_TYPED_DIGESTMAP_FNS(dsmap_, digest_ds_map_t, download_status_t) +DECLARE_TYPED_DIGESTMAP_FNS(dsmap, digest_ds_map_t, download_status_t) #define DSMAP_FOREACH(map, keyvar, valvar) \ DIGESTMAP_FOREACH(dsmap_to_digestmap(map), keyvar, download_status_t *, \ valvar) diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c index 42ce6f4c4e..f4e1215a40 100644 --- a/src/feature/nodelist/routerlist.c +++ b/src/feature/nodelist/routerlist.c @@ -117,9 +117,9 @@ /* Typed wrappers for different digestmap types; used to avoid type * confusion. */ -DECLARE_TYPED_DIGESTMAP_FNS(sdmap_, digest_sd_map_t, signed_descriptor_t) -DECLARE_TYPED_DIGESTMAP_FNS(rimap_, digest_ri_map_t, routerinfo_t) -DECLARE_TYPED_DIGESTMAP_FNS(eimap_, digest_ei_map_t, extrainfo_t) +DECLARE_TYPED_DIGESTMAP_FNS(sdmap, digest_sd_map_t, signed_descriptor_t) +DECLARE_TYPED_DIGESTMAP_FNS(rimap, digest_ri_map_t, routerinfo_t) +DECLARE_TYPED_DIGESTMAP_FNS(eimap, digest_ei_map_t, extrainfo_t) #define SDMAP_FOREACH(map, keyvar, valvar) \ DIGESTMAP_FOREACH(sdmap_to_digestmap(map), keyvar, signed_descriptor_t *, \ valvar) diff --git a/src/lib/container/map.h b/src/lib/container/map.h index 989ecfad80..dbc1967247 100644 --- a/src/lib/container/map.h +++ b/src/lib/container/map.h @@ -19,29 +19,29 @@ #define DECLARE_MAP_FNS(mapname_t, keytype, prefix) \ typedef struct mapname_t mapname_t; \ - typedef struct prefix##entry_t *prefix##iter_t; \ - MOCK_DECL(mapname_t*, prefix##new, (void)); \ - void* prefix##set(mapname_t *map, keytype key, void *val); \ - void* prefix##get(const mapname_t *map, keytype key); \ - void* prefix##remove(mapname_t *map, keytype key); \ - MOCK_DECL(void, prefix##free_, (mapname_t *map, void (*free_val)(void*))); \ - int prefix##isempty(const mapname_t *map); \ - int prefix##size(const mapname_t *map); \ - prefix##iter_t *prefix##iter_init(mapname_t *map); \ - prefix##iter_t *prefix##iter_next(mapname_t *map, prefix##iter_t *iter); \ - prefix##iter_t *prefix##iter_next_rmv(mapname_t *map, \ - prefix##iter_t *iter); \ - void prefix##iter_get(prefix##iter_t *iter, keytype *keyp, void **valp); \ - int prefix##iter_done(prefix##iter_t *iter); \ - void prefix##assert_ok(const mapname_t *map) + typedef struct prefix##_entry_t *prefix##_iter_t; \ + MOCK_DECL(mapname_t*, prefix##_new, (void)); \ + void* prefix##_set(mapname_t *map, keytype key, void *val); \ + void* prefix##_get(const mapname_t *map, keytype key); \ + void* prefix##_remove(mapname_t *map, keytype key); \ + MOCK_DECL(void, prefix##_free_, (mapname_t *map, void (*free_val)(void*))); \ + int prefix##_isempty(const mapname_t *map); \ + int prefix##_size(const mapname_t *map); \ + prefix##_iter_t *prefix##_iter_init(mapname_t *map); \ + prefix##_iter_t *prefix##_iter_next(mapname_t *map, prefix##_iter_t *iter); \ + prefix##_iter_t *prefix##_iter_next_rmv(mapname_t *map, \ + prefix##_iter_t *iter); \ + void prefix##_iter_get(prefix##_iter_t *iter, keytype *keyp, void **valp); \ + int prefix##_iter_done(prefix##_iter_t *iter); \ + void prefix##_assert_ok(const mapname_t *map) /* Map from const char * to void *. Implemented with a hash table. */ -DECLARE_MAP_FNS(strmap_t, const char *, strmap_); +DECLARE_MAP_FNS(strmap_t, const char *, strmap); /* Map from const char[DIGEST_LEN] to void *. Implemented with a hash table. */ -DECLARE_MAP_FNS(digestmap_t, const char *, digestmap_); +DECLARE_MAP_FNS(digestmap_t, const char *, digestmap); /* Map from const uint8_t[DIGEST256_LEN] to void *. Implemented with a hash * table. */ -DECLARE_MAP_FNS(digest256map_t, const uint8_t *, digest256map_); +DECLARE_MAP_FNS(digest256map_t, const uint8_t *, digest256map); #define MAP_FREE_AND_NULL(mapname_t, map, fn) \ do { \ @@ -56,12 +56,12 @@ DECLARE_MAP_FNS(digest256map_t, const uint8_t *, digest256map_); #undef DECLARE_MAP_FNS /** Iterates over the key-value pairs in a map map in order. - * prefix is as for DECLARE_MAP_FNS (i.e., strmap_ or digestmap_). + * prefix is as for DECLARE_MAP_FNS (i.e., strmap or digestmap). * The map's keys and values are of type keytype and valtype respectively; * each iteration assigns them to keyvar and valvar. * * Example use: - * MAP_FOREACH(digestmap_, m, const char *, k, routerinfo_t *, r) { + * MAP_FOREACH(digestmap, m, const char *, k, routerinfo_t *, r) { * // use k and r * } MAP_FOREACH_END. */ @@ -81,21 +81,21 @@ DECLARE_MAP_FNS(digest256map_t, const uint8_t *, digest256map_); */ #define MAP_FOREACH(prefix, map, keytype, keyvar, valtype, valvar) \ STMT_BEGIN \ - prefix##iter_t *keyvar##_iter; \ - for (keyvar##_iter = prefix##iter_init(map); \ - !prefix##iter_done(keyvar##_iter); \ - keyvar##_iter = prefix##iter_next(map, keyvar##_iter)) { \ + prefix##_iter_t *keyvar##_iter; \ + for (keyvar##_iter = prefix##_iter_init(map); \ + !prefix##_iter_done(keyvar##_iter); \ + keyvar##_iter = prefix##_iter_next(map, keyvar##_iter)) { \ keytype keyvar; \ void *valvar##_voidp; \ valtype valvar; \ - prefix##iter_get(keyvar##_iter, &keyvar, &valvar##_voidp); \ + prefix##_iter_get(keyvar##_iter, &keyvar, &valvar##_voidp); \ valvar = valvar##_voidp; /** As MAP_FOREACH, except allows members to be removed from the map * during the iteration via MAP_DEL_CURRENT. Example use: * * Example use: - * MAP_FOREACH(digestmap_, m, const char *, k, routerinfo_t *, r) { + * MAP_FOREACH(digestmap, m, const char *, k, routerinfo_t *, r) { * if (is_very_old(r)) * MAP_DEL_CURRENT(k); * } MAP_FOREACH_END. @@ -121,18 +121,18 @@ DECLARE_MAP_FNS(digest256map_t, const uint8_t *, digest256map_); */ #define MAP_FOREACH_MODIFY(prefix, map, keytype, keyvar, valtype, valvar) \ STMT_BEGIN \ - prefix##iter_t *keyvar##_iter; \ + prefix##_iter_t *keyvar##_iter; \ int keyvar##_del=0; \ - for (keyvar##_iter = prefix##iter_init(map); \ - !prefix##iter_done(keyvar##_iter); \ + for (keyvar##_iter = prefix##_iter_init(map); \ + !prefix##_iter_done(keyvar##_iter); \ keyvar##_iter = keyvar##_del ? \ - prefix##iter_next_rmv(map, keyvar##_iter) : \ - prefix##iter_next(map, keyvar##_iter)) { \ + prefix##_iter_next_rmv(map, keyvar##_iter) : \ + prefix##_iter_next(map, keyvar##_iter)) { \ keytype keyvar; \ void *valvar##_voidp; \ valtype valvar; \ keyvar##_del=0; \ - prefix##iter_get(keyvar##_iter, &keyvar, &valvar##_voidp); \ + prefix##_iter_get(keyvar##_iter, &keyvar, &valvar##_voidp); \ valvar = valvar##_voidp; /** Used with MAP_FOREACH_MODIFY to remove the currently-iterated-upon @@ -152,7 +152,7 @@ DECLARE_MAP_FNS(digest256map_t, const uint8_t *, digest256map_); * } DIGESTMAP_FOREACH_END. */ #define DIGESTMAP_FOREACH(map, keyvar, valtype, valvar) \ - MAP_FOREACH(digestmap_, map, const char *, keyvar, valtype, valvar) + MAP_FOREACH(digestmap, map, const char *, keyvar, valtype, valvar) /** As MAP_FOREACH_MODIFY, but does not require declaration of prefix or * keytype. @@ -163,21 +163,21 @@ DECLARE_MAP_FNS(digest256map_t, const uint8_t *, digest256map_); * } DIGESTMAP_FOREACH_END. */ #define DIGESTMAP_FOREACH_MODIFY(map, keyvar, valtype, valvar) \ - MAP_FOREACH_MODIFY(digestmap_, map, const char *, keyvar, valtype, valvar) + MAP_FOREACH_MODIFY(digestmap, map, const char *, keyvar, valtype, valvar) /** Used to end a DIGESTMAP_FOREACH() block. */ #define DIGESTMAP_FOREACH_END MAP_FOREACH_END #define DIGEST256MAP_FOREACH(map, keyvar, valtype, valvar) \ - MAP_FOREACH(digest256map_, map, const uint8_t *, keyvar, valtype, valvar) + MAP_FOREACH(digest256map, map, const uint8_t *, keyvar, valtype, valvar) #define DIGEST256MAP_FOREACH_MODIFY(map, keyvar, valtype, valvar) \ - MAP_FOREACH_MODIFY(digest256map_, map, const uint8_t *, \ + MAP_FOREACH_MODIFY(digest256map, map, const uint8_t *, \ keyvar, valtype, valvar) #define DIGEST256MAP_FOREACH_END MAP_FOREACH_END #define STRMAP_FOREACH(map, keyvar, valtype, valvar) \ - MAP_FOREACH(strmap_, map, const char *, keyvar, valtype, valvar) + MAP_FOREACH(strmap, map, const char *, keyvar, valtype, valvar) #define STRMAP_FOREACH_MODIFY(map, keyvar, valtype, valvar) \ - MAP_FOREACH_MODIFY(strmap_, map, const char *, keyvar, valtype, valvar) + MAP_FOREACH_MODIFY(strmap, map, const char *, keyvar, valtype, valvar) #define STRMAP_FOREACH_END MAP_FOREACH_END void* strmap_set_lc(strmap_t *map, const char *key, void *val); @@ -186,66 +186,66 @@ void* strmap_remove_lc(strmap_t *map, const char *key); #define DECLARE_TYPED_DIGESTMAP_FNS(prefix, mapname_t, valtype) \ typedef struct mapname_t mapname_t; \ - typedef struct prefix##iter_t *prefix##iter_t; \ + typedef struct prefix##_iter_t *prefix##_iter_t; \ ATTR_UNUSED static inline mapname_t* \ - prefix##new(void) \ + prefix##_new(void) \ { \ return (mapname_t*)digestmap_new(); \ } \ ATTR_UNUSED static inline digestmap_t* \ - prefix##to_digestmap(mapname_t *map) \ + prefix##_to_digestmap(mapname_t *map) \ { \ return (digestmap_t*)map; \ } \ ATTR_UNUSED static inline valtype* \ - prefix##get(mapname_t *map, const char *key) \ + prefix##_get(mapname_t *map, const char *key) \ { \ return (valtype*)digestmap_get((digestmap_t*)map, key); \ } \ ATTR_UNUSED static inline valtype* \ - prefix##set(mapname_t *map, const char *key, valtype *val) \ + prefix##_set(mapname_t *map, const char *key, valtype *val) \ { \ return (valtype*)digestmap_set((digestmap_t*)map, key, val); \ } \ ATTR_UNUSED static inline valtype* \ - prefix##remove(mapname_t *map, const char *key) \ + prefix##_remove(mapname_t *map, const char *key) \ { \ return (valtype*)digestmap_remove((digestmap_t*)map, key); \ } \ ATTR_UNUSED static inline void \ - prefix##f##ree_(mapname_t *map, void (*free_val)(void*)) \ + prefix##_f##ree_(mapname_t *map, void (*free_val)(void*)) \ { \ digestmap_free_((digestmap_t*)map, free_val); \ } \ ATTR_UNUSED static inline int \ - prefix##isempty(mapname_t *map) \ + prefix##_isempty(mapname_t *map) \ { \ return digestmap_isempty((digestmap_t*)map); \ } \ ATTR_UNUSED static inline int \ - prefix##size(mapname_t *map) \ + prefix##_size(mapname_t *map) \ { \ return digestmap_size((digestmap_t*)map); \ } \ ATTR_UNUSED static inline \ - prefix##iter_t *prefix##iter_init(mapname_t *map) \ + prefix##_iter_t *prefix##_iter_init(mapname_t *map) \ { \ - return (prefix##iter_t*) digestmap_iter_init((digestmap_t*)map); \ + return (prefix##_iter_t*) digestmap_iter_init((digestmap_t*)map); \ } \ ATTR_UNUSED static inline \ - prefix##iter_t *prefix##iter_next(mapname_t *map, prefix##iter_t *iter) \ + prefix##_iter_t *prefix##_iter_next(mapname_t *map, prefix##_iter_t *iter) \ { \ - return (prefix##iter_t*) digestmap_iter_next( \ + return (prefix##_iter_t*) digestmap_iter_next( \ (digestmap_t*)map, (digestmap_iter_t*)iter); \ } \ - ATTR_UNUSED static inline prefix##iter_t* \ - prefix##iter_next_rmv(mapname_t *map, prefix##iter_t *iter) \ + ATTR_UNUSED static inline prefix##_iter_t* \ + prefix##_iter_next_rmv(mapname_t *map, prefix##_iter_t *iter) \ { \ - return (prefix##iter_t*) digestmap_iter_next_rmv( \ + return (prefix##_iter_t*) digestmap_iter_next_rmv( \ (digestmap_t*)map, (digestmap_iter_t*)iter); \ } \ ATTR_UNUSED static inline void \ - prefix##iter_get(prefix##iter_t *iter, \ + prefix##_iter_get(prefix##_iter_t *iter, \ const char **keyp, \ valtype **valp) \ { \ @@ -254,7 +254,7 @@ void* strmap_remove_lc(strmap_t *map, const char *key); *valp = v; \ } \ ATTR_UNUSED static inline int \ - prefix##iter_done(prefix##iter_t *iter) \ + prefix##_iter_done(prefix##_iter_t *iter) \ { \ return digestmap_iter_done((digestmap_iter_t*)iter); \ } From 06a6130666315cb1d385b89d7c95df42ac17db1a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 15:01:42 -0500 Subject: [PATCH 16/28] Use parentheses to avoid mis-indentations of stringified macro args clang-format sometimes thinks that "#name" should be written as "# name" if it appears at the start of a line. Using () appears to suppress this, while confusing Coccinelle. --- src/app/config/config.c | 2 +- src/feature/control/control_cmd.c | 4 ++-- src/lib/conf/conftypes.h | 4 +++- src/test/test_pt.c | 4 +++- src/test/test_util.c | 18 +++++++++--------- src/test/test_util_process.c | 7 ++++--- 6 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index bbf984ad08..f75d68932e 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -182,7 +182,7 @@ static const char unix_q_socket_prefix[] = "unix:\""; * *DowloadInitialDelay . */ #ifndef COCCI #define DOWNLOAD_SCHEDULE(name) \ - { #name "DownloadSchedule", #name "DownloadInitialDelay", 0, 1 } + { (#name "DownloadSchedule"), (#name "DownloadInitialDelay"), 0, 1 } #else #define DOWNLOAD_SCHEDULE(name) { NULL, NULL, 0, 1 } #endif /* !defined(COCCI) */ diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index c2d23243e5..cdefef97e1 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -2272,7 +2272,7 @@ typedef struct control_cmd_def_t { **/ #define ONE_LINE(name, flags) \ { \ - #name, \ + (#name), \ handle_control_ ##name, \ flags, \ &name##_syntax, \ @@ -2283,7 +2283,7 @@ typedef struct control_cmd_def_t { * flags. **/ #define MULTLINE(name, flags) \ - { "+"#name, \ + { ("+"#name), \ handle_control_ ##name, \ flags, \ &name##_syntax \ diff --git a/src/lib/conf/conftypes.h b/src/lib/conf/conftypes.h index ebc2736aaa..081ebf397f 100644 --- a/src/lib/conf/conftypes.h +++ b/src/lib/conf/conftypes.h @@ -260,6 +260,7 @@ typedef struct config_deprecation_t { const char *why_deprecated; } config_deprecation_t; +#ifndef COCCI /** * Handy macro for declaring "In the config file or on the command line, you * can abbreviate toks as tok". Used inside an array of @@ -268,7 +269,8 @@ typedef struct config_deprecation_t { * For example, to declare "NumCpu" as an abbreviation for "NumCPUs", * you can say PLURAL(NumCpu). **/ -#define PLURAL(tok) { #tok, #tok "s", 0, 0 } +#define PLURAL(tok) { (#tok), (#tok "s"), 0, 0 } +#endif /* !defined(COCCI) */ /** * Validation function: verify whether a configuation object is well-formed diff --git a/src/test/test_pt.c b/src/test/test_pt.c index 26eaf7b7e7..893fec3674 100644 --- a/src/test/test_pt.c +++ b/src/test/test_pt.c @@ -579,8 +579,10 @@ test_get_pt_proxy_uri(void *arg) tor_free(uri); } +#ifndef COCCI #define PT_LEGACY(name) \ - { #name, test_pt_ ## name , 0, NULL, NULL } + { (#name), test_pt_ ## name , 0, NULL, NULL } +#endif struct testcase_t pt_tests[] = { PT_LEGACY(parsing), diff --git a/src/test/test_util.c b/src/test/test_util.c index 07c31f02d6..0226517d7b 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -6305,42 +6305,42 @@ test_util_map_anon_nofork(void *arg) #ifndef COCCI #define UTIL_LEGACY(name) \ - { #name, test_util_ ## name , 0, NULL, NULL } + { (#name), test_util_ ## name , 0, NULL, NULL } #define UTIL_TEST(name, flags) \ - { #name, test_util_ ## name, flags, NULL, NULL } + { (#name), test_util_ ## name, flags, NULL, NULL } #define COMPRESS(name, identifier) \ - { "compress/" #name, test_util_compress, 0, &compress_setup, \ + { ("compress/" #name), test_util_compress, 0, &compress_setup, \ (char*)(identifier) } #define COMPRESS_CONCAT(name, identifier) \ - { "compress_concat/" #name, test_util_decompress_concatenated, 0, \ + { ("compress_concat/" #name), test_util_decompress_concatenated, 0, \ &compress_setup, \ (char*)(identifier) } #define COMPRESS_JUNK(name, identifier) \ - { "compress_junk/" #name, test_util_decompress_junk, 0, \ + { ("compress_junk/" #name), test_util_decompress_junk, 0, \ &compress_setup, \ (char*)(identifier) } #define COMPRESS_DOS(name, identifier) \ - { "compress_dos/" #name, test_util_decompress_dos, 0, \ + { ("compress_dos/" #name), test_util_decompress_dos, 0, \ &compress_setup, \ (char*)(identifier) } -#endif /* !defined(COCCI) */ #ifdef _WIN32 #define UTIL_TEST_WIN_ONLY(n, f) UTIL_TEST(n, (f)) #else -#define UTIL_TEST_WIN_ONLY(n, f) { #n, NULL, TT_SKIP, NULL, NULL } +#define UTIL_TEST_WIN_ONLY(n, f) { (#n), NULL, TT_SKIP, NULL, NULL } #endif #ifdef DISABLE_PWDB_TESTS -#define UTIL_TEST_PWDB(n, f) { #n, NULL, TT_SKIP, NULL, NULL } +#define UTIL_TEST_PWDB(n, f) { (#n), NULL, TT_SKIP, NULL, NULL } #else #define UTIL_TEST_PWDB(n, f) UTIL_TEST(n, (f)) #endif +#endif struct testcase_t util_tests[] = { UTIL_LEGACY(time), diff --git a/src/test/test_util_process.c b/src/test/test_util_process.c index 0e17e009f3..f0e7334a9b 100644 --- a/src/test/test_util_process.c +++ b/src/test/test_util_process.c @@ -67,10 +67,12 @@ test_util_process_clear_waitpid_callback(void *ignored) } #endif /* !defined(_WIN32) */ +#ifndef COCCI #ifndef _WIN32 -#define TEST(name) { #name, test_util_process_##name, 0, NULL, NULL } +#define TEST(name) { (#name), test_util_process_##name, 0, NULL, NULL } #else -#define TEST(name) { #name, NULL, TT_SKIP, NULL, NULL } +#define TEST(name) { (#name), NULL, TT_SKIP, NULL, NULL } +#endif #endif struct testcase_t util_process_tests[] = { @@ -78,4 +80,3 @@ struct testcase_t util_process_tests[] = { TEST(clear_waitpid_callback), END_OF_TESTCASES }; - From 99a5aecbc779985629abce22dfc00a4e9d6ccb9e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 15:13:30 -0500 Subject: [PATCH 17/28] Wrap columnar tables in "clang-format off/on" These tables have aligned comments, so we don't want clang-format to mess with them. --- src/app/config/statefile.c | 2 ++ src/feature/client/entrynodes.c | 2 ++ src/feature/dirauth/shared_random_state.c | 2 ++ src/feature/dirparse/authcert_members.h | 2 ++ src/feature/dirparse/authcert_parse.c | 2 ++ src/feature/dirparse/microdesc_parse.c | 2 ++ src/feature/dirparse/ns_parse.c | 8 ++++++++ src/feature/dirparse/routerparse.c | 4 ++++ src/lib/confmgt/unitparse.c | 6 ++++++ src/lib/osinfo/uname.c | 2 ++ 10 files changed, 32 insertions(+) diff --git a/src/app/config/statefile.c b/src/app/config/statefile.c index d9667733cc..dcc55f1898 100644 --- a/src/app/config/statefile.c +++ b/src/app/config/statefile.c @@ -78,6 +78,7 @@ DUMMY_TYPECHECK_INSTANCE(or_state_t); VAR(#member, conftype, member, initvalue) /** Array of "state" variables saved to the ~/.tor/state file. */ +// clang-format off static const config_var_t state_vars_[] = { /* Remember to document these in state-contents.txt ! */ @@ -134,6 +135,7 @@ static const config_var_t state_vars_[] = { END_OF_CONFIG_VARS }; +// clang-format on #undef VAR #undef V diff --git a/src/feature/client/entrynodes.c b/src/feature/client/entrynodes.c index 8962f65006..2843558e93 100644 --- a/src/feature/client/entrynodes.c +++ b/src/feature/client/entrynodes.c @@ -1974,10 +1974,12 @@ get_retry_schedule(time_t failing_since, time_t now, const struct { time_t maximum; int primary_delay; int nonprimary_delay; } delays[] = { + // clang-format off { SIX_HOURS, 10*60, 1*60*60 }, { FOUR_DAYS, 90*60, 4*60*60 }, { SEVEN_DAYS, 4*60*60, 18*60*60 }, { TIME_MAX, 9*60*60, 36*60*60 } + // clang-format on }; unsigned i; diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c index 1792d540c6..52d9dc3448 100644 --- a/src/feature/dirauth/shared_random_state.c +++ b/src/feature/dirauth/shared_random_state.c @@ -60,6 +60,7 @@ DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t); #define SR_DISK_STATE_MAGIC 0x98AB1254 /** Array of variables that are saved to disk as a persistent state. */ +// clang-format off static const config_var_t state_vars[] = { V(Version, POSINT, "0"), V(TorVersion, STRING, NULL), @@ -73,6 +74,7 @@ static const config_var_t state_vars[] = { VAR("SharedRandCurrentValue", LINELIST_S, SharedRandValues, NULL), END_OF_CONFIG_VARS }; +// clang-format on /** "Extra" variable in the state that receives lines we can't parse. This * lets us preserve options from versions of Tor newer than us. */ diff --git a/src/feature/dirparse/authcert_members.h b/src/feature/dirparse/authcert_members.h index c6755bb629..53eab175d6 100644 --- a/src/feature/dirparse/authcert_members.h +++ b/src/feature/dirparse/authcert_members.h @@ -14,6 +14,7 @@ #ifndef TOR_AUTHCERT_MEMBERS_H #define TOR_AUTHCERT_MEMBERS_H +// clang-format off #define AUTHCERT_MEMBERS \ T1("dir-key-certificate-version", K_DIR_KEY_CERTIFICATE_VERSION, \ GE(1), NO_OBJ ), \ @@ -25,5 +26,6 @@ T1("dir-key-certification", K_DIR_KEY_CERTIFICATION,\ NO_ARGS, NEED_OBJ),\ T01("dir-address", K_DIR_ADDRESS, GE(1), NO_OBJ) +// clang-format on #endif /* !defined(TOR_AUTHCERT_MEMBERS_H) */ diff --git a/src/feature/dirparse/authcert_parse.c b/src/feature/dirparse/authcert_parse.c index 3d42119b94..deb45c12de 100644 --- a/src/feature/dirparse/authcert_parse.c +++ b/src/feature/dirparse/authcert_parse.c @@ -21,11 +21,13 @@ #include "feature/dirparse/authcert_members.h" /** List of tokens recognized in V3 authority certificates. */ +// clang-format off static token_rule_t dir_key_certificate_table[] = { AUTHCERT_MEMBERS, T1("fingerprint", K_FINGERPRINT, CONCAT_ARGS, NO_OBJ ), END_OF_TABLE }; +// clang-format on /** Parse a key certificate from s; point end-of-string to * the first character after the certificate. */ diff --git a/src/feature/dirparse/microdesc_parse.c b/src/feature/dirparse/microdesc_parse.c index c2eabeb404..9231080aaa 100644 --- a/src/feature/dirparse/microdesc_parse.c +++ b/src/feature/dirparse/microdesc_parse.c @@ -28,6 +28,7 @@ #include "feature/nodelist/microdesc_st.h" /** List of tokens recognized in microdescriptors */ +// clang-format off static token_rule_t microdesc_token_table[] = { T1_START("onion-key", K_ONION_KEY, NO_ARGS, NEED_KEY_1024), T01("ntor-onion-key", K_ONION_KEY_NTOR, GE(1), NO_OBJ ), @@ -39,6 +40,7 @@ static token_rule_t microdesc_token_table[] = { A01("@last-listed", A_LAST_LISTED, CONCAT_ARGS, NO_OBJ ), END_OF_TABLE }; +// clang-format on /** Assuming that s starts with a microdesc, return the start of the * *NEXT* one. Return NULL on "not found." */ diff --git a/src/feature/dirparse/ns_parse.c b/src/feature/dirparse/ns_parse.c index 4d9b6e6e73..ac9325a608 100644 --- a/src/feature/dirparse/ns_parse.c +++ b/src/feature/dirparse/ns_parse.c @@ -43,6 +43,7 @@ /** List of tokens recognized in the body part of v3 networkstatus * documents. */ +// clang-format off static token_rule_t rtrstatus_token_table[] = { T01("p", K_P, CONCAT_ARGS, NO_OBJ ), T1( "r", K_R, GE(7), NO_OBJ ), @@ -56,8 +57,10 @@ static token_rule_t rtrstatus_token_table[] = { T0N("opt", K_OPT, CONCAT_ARGS, OBJ_OK ), END_OF_TABLE }; +// clang-format on /** List of tokens recognized in V3 networkstatus votes. */ +// clang-format off static token_rule_t networkstatus_token_table[] = { T1_START("network-status-version", K_NETWORK_STATUS_VERSION, GE(1), NO_OBJ ), @@ -98,8 +101,10 @@ static token_rule_t networkstatus_token_table[] = { END_OF_TABLE }; +// clang-format on /** List of tokens recognized in V3 networkstatus consensuses. */ +// clang-format off static token_rule_t networkstatus_consensus_token_table[] = { T1_START("network-status-version", K_NETWORK_STATUS_VERSION, GE(1), NO_OBJ ), @@ -136,14 +141,17 @@ static token_rule_t networkstatus_consensus_token_table[] = { END_OF_TABLE }; +// clang-format on /** List of tokens recognized in the footer of v1 directory footers. */ +// clang-format off static token_rule_t networkstatus_vote_footer_token_table[] = { T01("directory-footer", K_DIRECTORY_FOOTER, NO_ARGS, NO_OBJ ), T01("bandwidth-weights", K_BW_WEIGHTS, ARGS, NO_OBJ ), T( "directory-signature", K_DIRECTORY_SIGNATURE, GE(2), NEED_OBJ ), END_OF_TABLE }; +// clang-format on /** Try to find the start and end of the signed portion of a networkstatus * document in s. On success, set start_out to the first diff --git a/src/feature/dirparse/routerparse.c b/src/feature/dirparse/routerparse.c index f476beec66..8828a0f97a 100644 --- a/src/feature/dirparse/routerparse.c +++ b/src/feature/dirparse/routerparse.c @@ -81,6 +81,7 @@ /****************************************************************************/ /** List of tokens recognized in router descriptors */ +// clang-format off const token_rule_t routerdesc_token_table[] = { T0N("reject", K_REJECT, ARGS, NO_OBJ ), T0N("accept", K_ACCEPT, ARGS, NO_OBJ ), @@ -123,8 +124,10 @@ const token_rule_t routerdesc_token_table[] = { END_OF_TABLE }; +// clang-format on /** List of tokens recognized in extra-info documents. */ +// clang-format off static token_rule_t extrainfo_token_table[] = { T1_END( "router-signature", K_ROUTER_SIGNATURE, NO_ARGS, NEED_OBJ ), T1( "published", K_PUBLISHED, CONCAT_ARGS, NO_OBJ ), @@ -162,6 +165,7 @@ static token_rule_t extrainfo_token_table[] = { END_OF_TABLE }; +// clang-format on #undef T diff --git a/src/lib/confmgt/unitparse.c b/src/lib/confmgt/unitparse.c index 52c0eabb69..995cb93834 100644 --- a/src/lib/confmgt/unitparse.c +++ b/src/lib/confmgt/unitparse.c @@ -21,6 +21,7 @@ /** Table to map the names of memory units to the number of bytes they * contain. */ +// clang-format off const struct unit_table_t memory_units[] = { { "", 1 }, { "b", 1<< 0 }, @@ -65,9 +66,11 @@ const struct unit_table_t memory_units[] = { { "tbit", UINT64_C(1)<<37 }, { NULL, 0 }, }; +// clang-format on /** Table to map the names of time units to the number of seconds they * contain. */ +// clang-format off const struct unit_table_t time_units[] = { { "", 1 }, { "second", 1 }, @@ -84,9 +87,11 @@ const struct unit_table_t time_units[] = { { "months", 2629728, }, { NULL, 0 }, }; +// clang-format on /** Table to map the names of time units to the number of milliseconds * they contain. */ +// clang-format off const struct unit_table_t time_msec_units[] = { { "", 1 }, { "msec", 1 }, @@ -104,6 +109,7 @@ const struct unit_table_t time_msec_units[] = { { "weeks", 7*24*60*60*1000 }, { NULL, 0 }, }; +// clang-format on /** Parse a string val containing a number, zero or more * spaces, and an optional unit string. If the unit appears in the diff --git a/src/lib/osinfo/uname.c b/src/lib/osinfo/uname.c index ac99726f51..66439d1336 100644 --- a/src/lib/osinfo/uname.c +++ b/src/lib/osinfo/uname.c @@ -61,6 +61,7 @@ get_uname,(void)) */ /* Windows Server 2019 is indistinguishable from Windows Server 2016 * using GetVersionEx(). +// clang-format off { 10, 0, NULL, "Windows Server 2019" }, */ { 10, 0, "Windows 10", "Windows Server 2016" }, { 6, 3, "Windows 8.1", "Windows Server 2012 R2" }, @@ -73,6 +74,7 @@ get_uname,(void)) { 5, 0, "Windows 2000 Professional", "Windows 2000 Server" }, /* Earlier versions are not supported by GetVersionEx(). */ { 0, 0, NULL, NULL } +// clang-format on }; memset(&info, 0, sizeof(info)); info.dwOSVersionInfoSize = sizeof(info); From f39ba52029c946304cc5a67fa93ca9aab10b2941 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 15:32:34 -0500 Subject: [PATCH 18/28] checkSpace: be more careful about bad function headers. Previously we would forbid macro indentations like this: FOO({ int x; }) But clang-format sometimes generates those. --- scripts/maint/checkSpace.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index bf1c69e00a..27615f910c 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -208,7 +208,7 @@ for my $fn (@ARGV) { ($fn !~ /\.h$/ && /^[a-zA-Z0-9_]/ && ! /^(?:const |static )*(?:typedef|struct|union)[^\(]*$/ && ! /= *\{$/ && ! /;$/) && ! /^[a-zA-Z0-9_]+\s*:/) { - if (/.\{$/){ + if (/[^,\s]\s*\{$/){ msg "fn() {:$fn:$.\n"; $in_func_head = 0; } elsif (/^\S[^\(]* +\**[a-zA-Z0-9_]+\(/) { From 6076adde256c7480b569f928c28d14ee637fbd6e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 10 Jan 2020 10:20:39 -0500 Subject: [PATCH 19/28] circuitbuild: expect bug message that clang-format will generate. clang-format wants to put no space here, so we need to tell the test to expect a lack of a space. --- src/feature/dirauth/dirvote.c | 2 +- src/test/test_dir.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 9490867e82..aec7a06c7a 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -886,7 +886,7 @@ dirvote_get_intermediate_param_value(const smartlist_t *param_list, int ok; value = (int32_t) tor_parse_long(integer_str, 10, INT32_MIN, INT32_MAX, &ok, NULL); - if (BUG(! ok)) + if (BUG(!ok)) return default_val; ++n_found; } diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 3b2ba64d2c..dd07e30e41 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -3022,7 +3022,7 @@ test_dir_param_voting_lookup(void *arg) dirvote_get_intermediate_param_value(lst, "jack", -100)); tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, - "!(! ok)"); + "!(!ok)"); tor_end_capture_bugs_(); /* electricity and opa aren't integers. */ tor_capture_bugs_(1); @@ -3030,7 +3030,7 @@ test_dir_param_voting_lookup(void *arg) dirvote_get_intermediate_param_value(lst, "electricity", -100)); tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, - "!(! ok)"); + "!(!ok)"); tor_end_capture_bugs_(); tor_capture_bugs_(1); @@ -3038,7 +3038,7 @@ test_dir_param_voting_lookup(void *arg) dirvote_get_intermediate_param_value(lst, "opa", -100)); tt_int_op(smartlist_len(tor_get_captured_bug_log_()), OP_EQ, 1); tt_str_op(smartlist_get(tor_get_captured_bug_log_(), 0), OP_EQ, - "!(! ok)"); + "!(!ok)"); tor_end_capture_bugs_(); done: From 7036ed34715678d64a9bfee99db69830f44d8912 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 6 Feb 2020 16:19:49 -0500 Subject: [PATCH 20/28] Fix a couple more long warning lines These are not a problem with 2-space indentation, but cocci will start getting confused when clang-format wraps them with 4-space indentation. --- src/lib/log/log.c | 2 +- src/lib/malloc/map_anon.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/log/log.c b/src/lib/log/log.c index 75f8f79da2..9ebe7350b2 100644 --- a/src/lib/log/log.c +++ b/src/lib/log/log.c @@ -523,7 +523,7 @@ logfile_deliver(logfile_t *lf, const char *buf, size_t msg_len, * pass them, and some very old ones do not detect overflow so well. * Regrettably, they call their maximum line length MAXLINE. */ #if MAXLINE < 64 -#warning "MAXLINE is a very low number; it might not be from syslog.h." +#warning "MAXLINE is very low; it might not be from syslog.h." #endif char *m = msg_after_prefix; if (msg_len >= MAXLINE) diff --git a/src/lib/malloc/map_anon.c b/src/lib/malloc/map_anon.c index 1926b61f07..628966012a 100644 --- a/src/lib/malloc/map_anon.c +++ b/src/lib/malloc/map_anon.c @@ -78,7 +78,7 @@ #endif /* defined(HAVE_MINHERIT) || ... */ #if defined(HAVE_MINHERIT) && !defined(FLAG_ZERO) && !defined(FLAG_NOINHERIT) -#warning "minherit() is defined, but we couldn't find the right flag for it." +#warning "minherit() is defined, but FLAG_ZERO/NOINHERIT are not." #warning "This is probably a bug in Tor's support for this platform." #endif From 5fa62fffee1972093798ed356884ef331dcd4ecc Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 6 Feb 2020 16:24:36 -0500 Subject: [PATCH 21/28] checkSpace: permit wide lines for LCOV_EXCL We're telling clang-format that a line with LCOV_EXCL must not be split -- that's fine, but we shouldn't complain when it indents it. --- scripts/maint/checkSpace.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/maint/checkSpace.pl b/scripts/maint/checkSpace.pl index 27615f910c..857ce6f6f1 100755 --- a/scripts/maint/checkSpace.pl +++ b/scripts/maint/checkSpace.pl @@ -130,7 +130,7 @@ for my $fn (@ARGV) { ## Terminals are still 80 columns wide in my world. I refuse to ## accept double-line lines. # (Don't make lines wider than 80 characters, including newline.) - if (/^.{80}/) { + if (/^.{80}/ and not /LCOV_EXCL/) { msg "Wide:$fn:$.\n"; } ### Juju to skip over comments and strings, since the tests From 8a5a1600cdbe678bbd271b4bff29bbe8b70d6b05 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 6 Feb 2020 17:02:43 -0500 Subject: [PATCH 22/28] Extract verbatim table in uname.c --- src/lib/osinfo/uname.c | 61 +++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/src/lib/osinfo/uname.c b/src/lib/osinfo/uname.c index 66439d1336..800acdddb6 100644 --- a/src/lib/osinfo/uname.c +++ b/src/lib/osinfo/uname.c @@ -27,6 +27,40 @@ static char uname_result[256]; /** True iff uname_result is set. */ static int uname_result_is_set = 0; +#ifdef _WIN32 +/** Table to map claimed windows versions into human-readable windows + * versions. */ +static struct { + unsigned major; + unsigned minor; + const char *client_version; + const char *server_version; +} win_version_table[] = { + /* This table must be sorted in descending order. + * Sources: + * https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions + * https://docs.microsoft.com/en-us/windows/desktop/api/winnt/ + * ns-winnt-_osversioninfoexa#remarks + */ + /* Windows Server 2019 is indistinguishable from Windows Server 2016 + * using GetVersionEx(). + { 10, 0, NULL, "Windows Server 2019" }, */ + // clang-format off + { 10, 0, "Windows 10", "Windows Server 2016" }, + { 6, 3, "Windows 8.1", "Windows Server 2012 R2" }, + { 6, 2, "Windows 8", "Windows Server 2012" }, + { 6, 1, "Windows 7", "Windows Server 2008 R2" }, + { 6, 0, "Windows Vista", "Windows Server 2008" }, + { 5, 2, "Windows XP Professional", "Windows Server 2003" }, + /* Windows XP did not have a server version, but we need something here */ + { 5, 1, "Windows XP", "Windows XP Server" }, + { 5, 0, "Windows 2000 Professional", "Windows 2000 Server" }, + /* Earlier versions are not supported by GetVersionEx(). */ + { 0, 0, NULL, NULL } + // clang-format on +}; +#endif + /** Return a pointer to a description of our platform. */ MOCK_IMPL(const char *, @@ -49,33 +83,6 @@ get_uname,(void)) int is_client = 0; int is_server = 0; const char *plat = NULL; - static struct { - unsigned major; unsigned minor; - const char *client_version; const char *server_version; - } win_version_table[] = { - /* This table must be sorted in descending order. - * Sources: - * https://en.wikipedia.org/wiki/List_of_Microsoft_Windows_versions - * https://docs.microsoft.com/en-us/windows/desktop/api/winnt/ - * ns-winnt-_osversioninfoexa#remarks - */ - /* Windows Server 2019 is indistinguishable from Windows Server 2016 - * using GetVersionEx(). -// clang-format off - { 10, 0, NULL, "Windows Server 2019" }, */ - { 10, 0, "Windows 10", "Windows Server 2016" }, - { 6, 3, "Windows 8.1", "Windows Server 2012 R2" }, - { 6, 2, "Windows 8", "Windows Server 2012" }, - { 6, 1, "Windows 7", "Windows Server 2008 R2" }, - { 6, 0, "Windows Vista", "Windows Server 2008" }, - { 5, 2, "Windows XP Professional", "Windows Server 2003" }, - /* Windows XP did not have a server version, but we need something here */ - { 5, 1, "Windows XP", "Windows XP Server" }, - { 5, 0, "Windows 2000 Professional", "Windows 2000 Server" }, - /* Earlier versions are not supported by GetVersionEx(). */ - { 0, 0, NULL, NULL } -// clang-format on - }; memset(&info, 0, sizeof(info)); info.dwOSVersionInfoSize = sizeof(info); if (! GetVersionEx((LPOSVERSIONINFO)&info)) { From 1651f92c1608c5df2c0e83ae377c86d14499eb34 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 6 Feb 2020 17:05:53 -0500 Subject: [PATCH 23/28] Break CONNECTION_TESTCAE_ARG across multiple lines --- src/test/test_connection.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/test/test_connection.c b/src/test/test_connection.c index 7ed831f7d8..fca4d884bb 100644 --- a/src/test/test_connection.c +++ b/src/test/test_connection.c @@ -967,9 +967,14 @@ test_failed_orconn_tracker(void *arg) #define CONNECTION_TESTCASE(name, fork, setup) \ { #name, test_conn_##name, fork, &setup, NULL } +#define STR(x) #x /* where arg is an expression (constant, variable, compound expression) */ -#define CONNECTION_TESTCASE_ARG(name, fork, setup, arg) \ - { #name "_" #arg, test_conn_##name, fork, &setup, (void *)arg } +#define CONNECTION_TESTCASE_ARG(name, fork, setup, arg) \ + { #name "_" STR(x), \ + test_conn_##name, \ + fork, \ + &setup, \ + (void *)arg } #endif /* !defined(COCCI) */ static const unsigned int PROXY_CONNECT_ARG = PROXY_CONNECT; From fbc1eaa0af653b796a794242010f54343141e272 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 6 Feb 2020 17:09:20 -0500 Subject: [PATCH 24/28] Try to shorten an #error in address.c --- src/lib/net/address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/net/address.c b/src/lib/net/address.c index ac4350b8fe..e968d8a7f7 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -97,7 +97,7 @@ * work correctly. Bail out here if we've found a platform where AF_UNSPEC * isn't 0. */ #if AF_UNSPEC != 0 -#error We rely on AF_UNSPEC being 0. Let us know about your platform, please! +#error "We rely on AF_UNSPEC being 0. Yours isn't. Please tell us more!" #endif CTASSERT(AF_UNSPEC == 0); From a9cc4ce0eb916255b447b7943c3a72a9feaccff9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 10 Feb 2020 12:54:06 -0500 Subject: [PATCH 25/28] ht.h: Require a semicolon after HT_PROTOTYPE and HT_GENERATE[2] --- src/ext/ht.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ext/ht.h b/src/ext/ht.h index 9d4add1936..4bfce36903 100644 --- a/src/ext/ht.h +++ b/src/ext/ht.h @@ -232,6 +232,10 @@ ht_string_hash(const char *s) #define HT_ASSERT_(x) (void)0 #endif +/* Macro put at the end of the end of a macro definition so that it + * consumes the following semicolon at file scope. Used only inside ht.h. */ +#define HT_EAT_SEMICOLON__ struct ht_semicolon_eater + #define HT_PROTOTYPE(name, type, field, hashfn, eqfn) \ int name##_HT_GROW(struct name *ht, unsigned min_capacity); \ void name##_HT_CLEAR(struct name *ht); \ @@ -413,7 +417,8 @@ ht_string_hash(const char *s) } \ return NULL; \ } \ - } + } \ + HT_EAT_SEMICOLON__ #define HT_GENERATE2(name, type, field, hashfn, eqfn, load, reallocarrayfn, \ freefn) \ @@ -538,7 +543,8 @@ ht_string_hash(const char *s) if (n != head->hth_n_entries) \ return 6; \ return 0; \ - } + } \ + HT_EAT_SEMICOLON__ #define HT_GENERATE(name, type, field, hashfn, eqfn, load, mallocfn, \ reallocfn, freefn) \ From d9e211ab7025acce3eb3a96d85791caff52ce095 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 10 Feb 2020 12:54:43 -0500 Subject: [PATCH 26/28] Use semicolons after HT_PROTOTYPE and HT_GENERATE. --- src/core/or/channel.c | 8 ++++---- src/core/or/circuitlist.c | 4 ++-- src/core/or/circuitmux.c | 5 ++--- src/core/or/connection_or.c | 4 ++-- src/core/or/policies.c | 4 ++-- src/core/or/scheduler_kist.c | 8 ++++---- src/feature/control/btrack_orconn_maps.c | 9 +++++---- src/feature/dirauth/dircollate.c | 5 ++--- src/feature/dirauth/keypin.c | 8 ++++---- src/feature/dircache/consdiffmgr.c | 4 ++-- src/feature/dircommon/fp_pair.c | 5 ++--- src/feature/hs/hs_circuitmap.c | 4 ++-- src/feature/hs/hs_service.c | 4 ++-- src/feature/nodelist/microdesc.c | 4 ++-- src/feature/nodelist/nodefamily.c | 4 ++-- src/feature/nodelist/nodelist.c | 8 ++++---- src/feature/relay/dns.c | 4 ++-- src/feature/stats/geoip_stats.c | 8 ++++---- src/feature/stats/rephist.c | 4 ++-- src/lib/container/map.c | 12 ++++++------ src/lib/container/namemap.c | 4 ++-- src/lib/net/resolve.c | 4 ++-- src/lib/process/waitpid.c | 4 ++-- 23 files changed, 63 insertions(+), 65 deletions(-) diff --git a/src/core/or/channel.c b/src/core/or/channel.c index d52dc14a32..ce1c31914c 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -119,10 +119,10 @@ channel_id_eq(const channel_t *a, const channel_t *b) return a->global_identifier == b->global_identifier; } HT_PROTOTYPE(channel_gid_map, channel_t, gidmap_node, - channel_id_hash, channel_id_eq) + channel_id_hash, channel_id_eq); HT_GENERATE2(channel_gid_map, channel_t, gidmap_node, channel_id_hash, channel_id_eq, - 0.6, tor_reallocarray_, tor_free_) + 0.6, tor_reallocarray_, tor_free_); HANDLE_IMPL(channel, channel_t,) @@ -160,9 +160,9 @@ channel_idmap_eq(const channel_idmap_entry_t *a, } HT_PROTOTYPE(channel_idmap, channel_idmap_entry_t, node, channel_idmap_hash, - channel_idmap_eq) + channel_idmap_eq); HT_GENERATE2(channel_idmap, channel_idmap_entry_t, node, channel_idmap_hash, - channel_idmap_eq, 0.5, tor_reallocarray_, tor_free_) + channel_idmap_eq, 0.5, tor_reallocarray_, tor_free_); /* Functions to maintain the digest map */ static void channel_remove_from_digest_map(channel_t *chan); diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c index 6a712926a3..ca174c442c 100644 --- a/src/core/or/circuitlist.c +++ b/src/core/or/circuitlist.c @@ -215,10 +215,10 @@ chan_circid_entry_hash_(chan_circid_circuit_map_t *a) static HT_HEAD(chan_circid_map, chan_circid_circuit_map_t) chan_circid_map = HT_INITIALIZER(); HT_PROTOTYPE(chan_circid_map, chan_circid_circuit_map_t, node, - chan_circid_entry_hash_, chan_circid_entries_eq_) + chan_circid_entry_hash_, chan_circid_entries_eq_); HT_GENERATE2(chan_circid_map, chan_circid_circuit_map_t, node, chan_circid_entry_hash_, chan_circid_entries_eq_, 0.6, - tor_reallocarray_, tor_free_) + tor_reallocarray_, tor_free_); /** The most recently returned entry from circuit_get_by_circid_chan; * used to improve performance when many cells arrive in a row from the diff --git a/src/core/or/circuitmux.c b/src/core/or/circuitmux.c index 0e932f032d..d9859bfc90 100644 --- a/src/core/or/circuitmux.c +++ b/src/core/or/circuitmux.c @@ -174,10 +174,10 @@ chanid_circid_entry_hash(chanid_circid_muxinfo_t *a) /* Emit a bunch of hash table stuff */ HT_PROTOTYPE(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t, node, - chanid_circid_entry_hash, chanid_circid_entries_eq) + chanid_circid_entry_hash, chanid_circid_entries_eq); HT_GENERATE2(chanid_circid_muxinfo_map, chanid_circid_muxinfo_t, node, chanid_circid_entry_hash, chanid_circid_entries_eq, 0.6, - tor_reallocarray_, tor_free_) + tor_reallocarray_, tor_free_); /* * Circuitmux alloc/free functions @@ -1282,4 +1282,3 @@ circuitmux_compare_muxes, (circuitmux_t *cmux_1, circuitmux_t *cmux_2)) return 0; } } - diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c index 76bfbf0b30..d315241c99 100644 --- a/src/core/or/connection_or.c +++ b/src/core/or/connection_or.c @@ -1283,11 +1283,11 @@ or_connect_failure_ht_hash(const or_connect_failure_entry_t *entry) } HT_PROTOTYPE(or_connect_failure_ht, or_connect_failure_entry_t, node, - or_connect_failure_ht_hash, or_connect_failure_ht_eq) + or_connect_failure_ht_hash, or_connect_failure_ht_eq); HT_GENERATE2(or_connect_failure_ht, or_connect_failure_entry_t, node, or_connect_failure_ht_hash, or_connect_failure_ht_eq, - 0.6, tor_reallocarray_, tor_free_) + 0.6, tor_reallocarray_, tor_free_); /* Initialize a given connect failure entry with the given identity_digest, * addr and port. All field are optional except ocf. */ diff --git a/src/core/or/policies.c b/src/core/or/policies.c index a82995fe12..4ac598fce3 100644 --- a/src/core/or/policies.c +++ b/src/core/or/policies.c @@ -1405,9 +1405,9 @@ policy_hash(const policy_map_ent_t *ent) } HT_PROTOTYPE(policy_map, policy_map_ent_t, node, policy_hash, - policy_eq) + policy_eq); HT_GENERATE2(policy_map, policy_map_ent_t, node, policy_hash, - policy_eq, 0.6, tor_reallocarray_, tor_free_) + policy_eq, 0.6, tor_reallocarray_, tor_free_); /** Given a pointer to an addr_policy_t, return a copy of the pointer to the * "canonical" copy of that addr_policy_t; the canonical copy is a single diff --git a/src/core/or/scheduler_kist.c b/src/core/or/scheduler_kist.c index e56942be09..113fb285f7 100644 --- a/src/core/or/scheduler_kist.c +++ b/src/core/or/scheduler_kist.c @@ -56,9 +56,9 @@ typedef HT_HEAD(socket_table_s, socket_table_ent_t) socket_table_t; static socket_table_t socket_table = HT_INITIALIZER(); HT_PROTOTYPE(socket_table_s, socket_table_ent_t, node, socket_table_ent_hash, - socket_table_ent_eq) + socket_table_ent_eq); HT_GENERATE2(socket_table_s, socket_table_ent_t, node, socket_table_ent_hash, - socket_table_ent_eq, 0.6, tor_reallocarray, tor_free_) + socket_table_ent_eq, 0.6, tor_reallocarray, tor_free_); /* outbuf_table hash table stuff. The outbuf_table keeps track of which * channels have data sitting in their outbuf so the kist scheduler can force @@ -83,9 +83,9 @@ outbuf_table_ent_eq(const outbuf_table_ent_t *a, const outbuf_table_ent_t *b) } HT_PROTOTYPE(outbuf_table_s, outbuf_table_ent_t, node, outbuf_table_ent_hash, - outbuf_table_ent_eq) + outbuf_table_ent_eq); HT_GENERATE2(outbuf_table_s, outbuf_table_ent_t, node, outbuf_table_ent_hash, - outbuf_table_ent_eq, 0.6, tor_reallocarray, tor_free_) + outbuf_table_ent_eq, 0.6, tor_reallocarray, tor_free_); /***************************************************************************** * Other internal data diff --git a/src/feature/control/btrack_orconn_maps.c b/src/feature/control/btrack_orconn_maps.c index 0ef54237a8..a60dffb8c4 100644 --- a/src/feature/control/btrack_orconn_maps.c +++ b/src/feature/control/btrack_orconn_maps.c @@ -47,17 +47,18 @@ bto_chan_eq_(bt_orconn_t *a, bt_orconn_t *b) } HT_HEAD(bto_gid_ht, bt_orconn_t); -HT_PROTOTYPE(bto_gid_ht, bt_orconn_t, node, bto_gid_hash_, bto_gid_eq_) +HT_PROTOTYPE(bto_gid_ht, bt_orconn_t, node, bto_gid_hash_, bto_gid_eq_); HT_GENERATE2(bto_gid_ht, bt_orconn_t, node, bto_gid_hash_, bto_gid_eq_, 0.6, - tor_reallocarray_, tor_free_) + tor_reallocarray_, tor_free_); static struct bto_gid_ht *bto_gid_map; HT_HEAD(bto_chan_ht, bt_orconn_t); -HT_PROTOTYPE(bto_chan_ht, bt_orconn_t, chan_node, bto_chan_hash_, bto_chan_eq_) +HT_PROTOTYPE(bto_chan_ht, bt_orconn_t, chan_node, bto_chan_hash_, + bto_chan_eq_); HT_GENERATE2(bto_chan_ht, bt_orconn_t, chan_node, bto_chan_hash_, bto_chan_eq_, 0.6, - tor_reallocarray_, tor_free_) + tor_reallocarray_, tor_free_); static struct bto_chan_ht *bto_chan_map; /** Clear the GID hash map, freeing any bt_orconn_t objects that become diff --git a/src/feature/dirauth/dircollate.c b/src/feature/dirauth/dircollate.c index b35cb021ff..2657f53853 100644 --- a/src/feature/dirauth/dircollate.c +++ b/src/feature/dirauth/dircollate.c @@ -90,9 +90,9 @@ ddmap_entry_set_digests(ddmap_entry_t *ent, } HT_PROTOTYPE(double_digest_map, ddmap_entry_t, node, ddmap_entry_hash, - ddmap_entry_eq) + ddmap_entry_eq); HT_GENERATE2(double_digest_map, ddmap_entry_t, node, ddmap_entry_hash, - ddmap_entry_eq, 0.6, tor_reallocarray, tor_free_) + ddmap_entry_eq, 0.6, tor_reallocarray, tor_free_); /** Helper: add a single vote_routerstatus_t vrs to the collator * dc, indexing it by its RSA key digest, and by the 2-tuple of its RSA @@ -324,4 +324,3 @@ dircollator_get_votes_for_router(dircollator_t *dc, int idx) return digestmap_get(dc->by_collated_rsa_sha1, smartlist_get(dc->all_rsa_sha1_lst, idx)); } - diff --git a/src/feature/dirauth/keypin.c b/src/feature/dirauth/keypin.c index 6f6cfc01f1..98584a7d42 100644 --- a/src/feature/dirauth/keypin.c +++ b/src/feature/dirauth/keypin.c @@ -118,14 +118,14 @@ return (unsigned) siphash24g(a->ed25519_key, sizeof(a->ed25519_key)); } HT_PROTOTYPE(rsamap, keypin_ent_st, rsamap_node, keypin_ent_hash_rsa, - keypin_ents_eq_rsa) + keypin_ents_eq_rsa); HT_GENERATE2(rsamap, keypin_ent_st, rsamap_node, keypin_ent_hash_rsa, - keypin_ents_eq_rsa, 0.6, tor_reallocarray, tor_free_) + keypin_ents_eq_rsa, 0.6, tor_reallocarray, tor_free_); HT_PROTOTYPE(edmap, keypin_ent_st, edmap_node, keypin_ent_hash_ed, - keypin_ents_eq_ed) + keypin_ents_eq_ed); HT_GENERATE2(edmap, keypin_ent_st, edmap_node, keypin_ent_hash_ed, - keypin_ents_eq_ed, 0.6, tor_reallocarray, tor_free_) + keypin_ents_eq_ed, 0.6, tor_reallocarray, tor_free_); /** * Check whether we already have an entry in the key pinning table for a diff --git a/src/feature/dircache/consdiffmgr.c b/src/feature/dircache/consdiffmgr.c index 8445b8f986..10590cd6d2 100644 --- a/src/feature/dircache/consdiffmgr.c +++ b/src/feature/dircache/consdiffmgr.c @@ -218,9 +218,9 @@ cdm_diff_eq(const cdm_diff_t *diff1, const cdm_diff_t *diff2) diff1->compress_method == diff2->compress_method; } -HT_PROTOTYPE(cdm_diff_ht, cdm_diff_t, node, cdm_diff_hash, cdm_diff_eq) +HT_PROTOTYPE(cdm_diff_ht, cdm_diff_t, node, cdm_diff_hash, cdm_diff_eq); HT_GENERATE2(cdm_diff_ht, cdm_diff_t, node, cdm_diff_hash, cdm_diff_eq, - 0.6, tor_reallocarray, tor_free_) + 0.6, tor_reallocarray, tor_free_); #define cdm_diff_free(diff) \ FREE_AND_NULL(cdm_diff_t, cdm_diff_free_, (diff)) diff --git a/src/feature/dircommon/fp_pair.c b/src/feature/dircommon/fp_pair.c index 8b55896ba8..87e1c253bd 100644 --- a/src/feature/dircommon/fp_pair.c +++ b/src/feature/dircommon/fp_pair.c @@ -57,10 +57,10 @@ fp_pair_map_entry_hash(const fp_pair_map_entry_t *a) */ HT_PROTOTYPE(fp_pair_map_impl, fp_pair_map_entry_t, node, - fp_pair_map_entry_hash, fp_pair_map_entries_eq) + fp_pair_map_entry_hash, fp_pair_map_entries_eq); HT_GENERATE2(fp_pair_map_impl, fp_pair_map_entry_t, node, fp_pair_map_entry_hash, fp_pair_map_entries_eq, - 0.6, tor_reallocarray_, tor_free_) + 0.6, tor_reallocarray_, tor_free_); /** Constructor to create a new empty map from fp_pair_t to void * */ @@ -312,4 +312,3 @@ fp_pair_map_assert_ok(const fp_pair_map_t *map) { tor_assert(!fp_pair_map_impl_HT_REP_IS_BAD_(&(map->head))); } - diff --git a/src/feature/hs/hs_circuitmap.c b/src/feature/hs/hs_circuitmap.c index 2343d729dd..466a02de39 100644 --- a/src/feature/hs/hs_circuitmap.c +++ b/src/feature/hs/hs_circuitmap.c @@ -76,11 +76,11 @@ hs_circuit_hash_token(const circuit_t *circuit) HT_PROTOTYPE(hs_circuitmap_ht, // The name of the hashtable struct circuit_t, // The name of the element struct, hs_circuitmap_node, // The name of HT_ENTRY member - hs_circuit_hash_token, hs_circuits_have_same_token) + hs_circuit_hash_token, hs_circuits_have_same_token); HT_GENERATE2(hs_circuitmap_ht, circuit_t, hs_circuitmap_node, hs_circuit_hash_token, hs_circuits_have_same_token, - 0.6, tor_reallocarray, tor_free_) + 0.6, tor_reallocarray, tor_free_); #ifdef TOR_UNIT_TESTS diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 81b37eab40..7cf6d1f152 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -151,11 +151,11 @@ HT_PROTOTYPE(hs_service_ht, /* Name of hashtable. */ hs_service_t, /* Object contained in the map. */ hs_service_node, /* The name of the HT_ENTRY member. */ hs_service_ht_hash, /* Hashing function. */ - hs_service_ht_eq) /* Compare function for objects. */ + hs_service_ht_eq); /* Compare function for objects. */ HT_GENERATE2(hs_service_ht, hs_service_t, hs_service_node, hs_service_ht_hash, hs_service_ht_eq, - 0.6, tor_reallocarray, tor_free_) + 0.6, tor_reallocarray, tor_free_); /** Query the given service map with a public key and return a service object * if found else NULL. It is also possible to set a directory path in the diff --git a/src/feature/nodelist/microdesc.c b/src/feature/nodelist/microdesc.c index d32a4ea61e..cf7732b8dc 100644 --- a/src/feature/nodelist/microdesc.c +++ b/src/feature/nodelist/microdesc.c @@ -90,10 +90,10 @@ microdesc_eq_(microdesc_t *a, microdesc_t *b) } HT_PROTOTYPE(microdesc_map, microdesc_t, node, - microdesc_hash_, microdesc_eq_) + microdesc_hash_, microdesc_eq_); HT_GENERATE2(microdesc_map, microdesc_t, node, microdesc_hash_, microdesc_eq_, 0.6, - tor_reallocarray_, tor_free_) + tor_reallocarray_, tor_free_); /************************* md fetch fail cache *****************************/ diff --git a/src/feature/nodelist/nodefamily.c b/src/feature/nodelist/nodefamily.c index 7ae8620749..feaa3730dc 100644 --- a/src/feature/nodelist/nodefamily.c +++ b/src/feature/nodelist/nodefamily.c @@ -69,9 +69,9 @@ static HT_HEAD(nodefamily_map, nodefamily_t) the_node_families = HT_INITIALIZER(); HT_PROTOTYPE(nodefamily_map, nodefamily_t, ht_ent, nodefamily_hash, - nodefamily_eq) + nodefamily_eq); HT_GENERATE2(nodefamily_map, nodefamily_t, ht_ent, nodefamily_hash, - node_family_eq, 0.6, tor_reallocarray_, tor_free_) + node_family_eq, 0.6, tor_reallocarray_, tor_free_); /** * Parse the family declaration in s, returning the canonical diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 94ff08826f..91bf3175e2 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -153,9 +153,9 @@ node_id_eq(const node_t *node1, const node_t *node2) return tor_memeq(node1->identity, node2->identity, DIGEST_LEN); } -HT_PROTOTYPE(nodelist_map, node_t, ht_ent, node_id_hash, node_id_eq) +HT_PROTOTYPE(nodelist_map, node_t, ht_ent, node_id_hash, node_id_eq); HT_GENERATE2(nodelist_map, node_t, ht_ent, node_id_hash, node_id_eq, - 0.6, tor_reallocarray_, tor_free_) + 0.6, tor_reallocarray_, tor_free_); static inline unsigned int node_ed_id_hash(const node_t *node) @@ -170,9 +170,9 @@ node_ed_id_eq(const node_t *node1, const node_t *node2) } HT_PROTOTYPE(nodelist_ed_map, node_t, ed_ht_ent, node_ed_id_hash, - node_ed_id_eq) + node_ed_id_eq); HT_GENERATE2(nodelist_ed_map, node_t, ed_ht_ent, node_ed_id_hash, - node_ed_id_eq, 0.6, tor_reallocarray_, tor_free_) + node_ed_id_eq, 0.6, tor_reallocarray_, tor_free_); /** The global nodelist. */ static nodelist_t *the_nodelist=NULL; diff --git a/src/feature/relay/dns.c b/src/feature/relay/dns.c index da0cbb1df4..3d732896b1 100644 --- a/src/feature/relay/dns.c +++ b/src/feature/relay/dns.c @@ -146,9 +146,9 @@ cached_resolve_hash(cached_resolve_t *a) } HT_PROTOTYPE(cache_map, cached_resolve_t, node, cached_resolve_hash, - cached_resolves_eq) + cached_resolves_eq); HT_GENERATE2(cache_map, cached_resolve_t, node, cached_resolve_hash, - cached_resolves_eq, 0.6, tor_reallocarray_, tor_free_) + cached_resolves_eq, 0.6, tor_reallocarray_, tor_free_); /** Initialize the DNS cache. */ static void diff --git a/src/feature/stats/geoip_stats.c b/src/feature/stats/geoip_stats.c index 3228b18973..f9a2f19d2e 100644 --- a/src/feature/stats/geoip_stats.c +++ b/src/feature/stats/geoip_stats.c @@ -146,9 +146,9 @@ clientmap_entries_eq(const clientmap_entry_t *a, const clientmap_entry_t *b) } HT_PROTOTYPE(clientmap, clientmap_entry_t, node, clientmap_entry_hash, - clientmap_entries_eq) + clientmap_entries_eq); HT_GENERATE2(clientmap, clientmap_entry_t, node, clientmap_entry_hash, - clientmap_entries_eq, 0.6, tor_reallocarray_, tor_free_) + clientmap_entries_eq, 0.6, tor_reallocarray_, tor_free_); #define clientmap_entry_free(ent) \ FREE_AND_NULL(clientmap_entry_t, clientmap_entry_free_, ent) @@ -484,9 +484,9 @@ dirreq_map_ent_hash(const dirreq_map_entry_t *entry) } HT_PROTOTYPE(dirreqmap, dirreq_map_entry_t, node, dirreq_map_ent_hash, - dirreq_map_ent_eq) + dirreq_map_ent_eq); HT_GENERATE2(dirreqmap, dirreq_map_entry_t, node, dirreq_map_ent_hash, - dirreq_map_ent_eq, 0.6, tor_reallocarray_, tor_free_) + dirreq_map_ent_eq, 0.6, tor_reallocarray_, tor_free_); /** Helper: Put entry into map of directory requests using * type and dirreq_id as key parts. If there is diff --git a/src/feature/stats/rephist.c b/src/feature/stats/rephist.c index b2817ee760..5ada142ca7 100644 --- a/src/feature/stats/rephist.c +++ b/src/feature/stats/rephist.c @@ -2285,9 +2285,9 @@ bidi_map_ent_hash(const bidi_map_entry_t *entry) } HT_PROTOTYPE(bidimap, bidi_map_entry_t, node, bidi_map_ent_hash, - bidi_map_ent_eq) + bidi_map_ent_eq); HT_GENERATE2(bidimap, bidi_map_entry_t, node, bidi_map_ent_hash, - bidi_map_ent_eq, 0.6, tor_reallocarray_, tor_free_) + bidi_map_ent_eq, 0.6, tor_reallocarray_, tor_free_); /* DOCDOC bidi_map_free */ static void diff --git a/src/lib/container/map.c b/src/lib/container/map.c index c3fb0b5c8a..7db84313ea 100644 --- a/src/lib/container/map.c +++ b/src/lib/container/map.c @@ -85,21 +85,21 @@ digest256map_entry_hash(const digest256map_entry_t *a) } HT_PROTOTYPE(strmap_impl, strmap_entry_t, node, strmap_entry_hash, - strmap_entries_eq) + strmap_entries_eq); HT_GENERATE2(strmap_impl, strmap_entry_t, node, strmap_entry_hash, - strmap_entries_eq, 0.6, tor_reallocarray_, tor_free_) + strmap_entries_eq, 0.6, tor_reallocarray_, tor_free_); HT_PROTOTYPE(digestmap_impl, digestmap_entry_t, node, digestmap_entry_hash, - digestmap_entries_eq) + digestmap_entries_eq); HT_GENERATE2(digestmap_impl, digestmap_entry_t, node, digestmap_entry_hash, - digestmap_entries_eq, 0.6, tor_reallocarray_, tor_free_) + digestmap_entries_eq, 0.6, tor_reallocarray_, tor_free_); HT_PROTOTYPE(digest256map_impl, digest256map_entry_t, node, digest256map_entry_hash, - digest256map_entries_eq) + digest256map_entries_eq); HT_GENERATE2(digest256map_impl, digest256map_entry_t, node, digest256map_entry_hash, - digest256map_entries_eq, 0.6, tor_reallocarray_, tor_free_) + digest256map_entries_eq, 0.6, tor_reallocarray_, tor_free_); #define strmap_entry_free(ent) \ FREE_AND_NULL(strmap_entry_t, strmap_entry_free_, (ent)) diff --git a/src/lib/container/namemap.c b/src/lib/container/namemap.c index 28695ee3a1..e286cad947 100644 --- a/src/lib/container/namemap.c +++ b/src/lib/container/namemap.c @@ -35,9 +35,9 @@ mapped_name_hash(const mapped_name_t *a) } HT_PROTOTYPE(namemap_ht, mapped_name_t, node, mapped_name_hash, - mapped_name_eq) + mapped_name_eq); HT_GENERATE2(namemap_ht, mapped_name_t, node, mapped_name_hash, - mapped_name_eq, 0.6, tor_reallocarray_, tor_free_) + mapped_name_eq, 0.6, tor_reallocarray_, tor_free_); /** Set up an uninitialized map. */ void diff --git a/src/lib/net/resolve.c b/src/lib/net/resolve.c index df079d5db3..68a8c01ef4 100644 --- a/src/lib/net/resolve.c +++ b/src/lib/net/resolve.c @@ -372,11 +372,11 @@ static HT_HEAD(getaddrinfo_cache, cached_getaddrinfo_item_t) HT_PROTOTYPE(getaddrinfo_cache, cached_getaddrinfo_item_t, node, cached_getaddrinfo_item_hash, - cached_getaddrinfo_items_eq) + cached_getaddrinfo_items_eq); HT_GENERATE2(getaddrinfo_cache, cached_getaddrinfo_item_t, node, cached_getaddrinfo_item_hash, cached_getaddrinfo_items_eq, - 0.6, tor_reallocarray_, tor_free_) + 0.6, tor_reallocarray_, tor_free_); /** If true, don't try to cache getaddrinfo results. */ static int sandbox_getaddrinfo_cache_disabled = 0; diff --git a/src/lib/process/waitpid.c b/src/lib/process/waitpid.c index 89ffe9fcfe..33798f65f0 100644 --- a/src/lib/process/waitpid.c +++ b/src/lib/process/waitpid.c @@ -58,9 +58,9 @@ process_map_entries_eq_(const waitpid_callback_t *a, static HT_HEAD(process_map, waitpid_callback_t) process_map = HT_INITIALIZER(); HT_PROTOTYPE(process_map, waitpid_callback_t, node, process_map_entry_hash_, - process_map_entries_eq_) + process_map_entries_eq_); HT_GENERATE2(process_map, waitpid_callback_t, node, process_map_entry_hash_, - process_map_entries_eq_, 0.6, tor_reallocarray_, tor_free_) + process_map_entries_eq_, 0.6, tor_reallocarray_, tor_free_); /** * Begin monitoring the child pid pid to see if we get a SIGCHLD for From 3b62cd85e2a862ecc964d24dfeb3d83ad8d652fe Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 10 Feb 2020 12:55:00 -0500 Subject: [PATCH 27/28] clang-format: remove StatementMacros usage This change lets us use clang-format-6.0, and is okay since we now require semicolons after HT_PROTOTYPE/GENERATE. --- .clang-format | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.clang-format b/.clang-format index 31b76a0025..e688ce7df5 100644 --- a/.clang-format +++ b/.clang-format @@ -148,14 +148,6 @@ IncludeCategories: MacroBlockBegin: "^STMT_BEGIN|TT_STMT_BEGIN$" MacroBlockEnd: "^STMT_END|TT_STMT_END$" -# -# These macros don't need to have semicolons afterwards. -# -StatementMacros: - - HT_PROTOTYPE - - HT_GENERATE - - HT_GENERATE2 - # # These macros are interpreted as types. # (Not supported in my clang-format) From b5ccdd978ea138cde92b3513c9d653ba18b8b463 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 12 Feb 2020 18:52:35 -0500 Subject: [PATCH 28/28] Add a dire warning about not reformatting the whole codebase yet. --- .clang-format | 5 +++++ scripts/maint/clang-format.sh | 7 +++++++ scripts/maint/codetool.py | 8 ++++++++ 3 files changed, 20 insertions(+) diff --git a/.clang-format b/.clang-format index e688ce7df5..7be73e0612 100644 --- a/.clang-format +++ b/.clang-format @@ -1,3 +1,8 @@ +# DO NOT COMMIT OR MERGE CODE THAT IS RUN THROUGH THIS TOOL YET. +# +# WE ARE STILL DISCUSSING OUR DESIRED STYLE AND ITERATING ON IT. +# (12 Feb 2020) + --- Language: Cpp # Out of all supported styles, LLVM seems closest to our own. diff --git a/scripts/maint/clang-format.sh b/scripts/maint/clang-format.sh index 86430b9b26..59832117b4 100755 --- a/scripts/maint/clang-format.sh +++ b/scripts/maint/clang-format.sh @@ -2,6 +2,13 @@ # Copyright 2020, The Tor Project, Inc. # See LICENSE for licensing information. +# +# DO NOT COMMIT OR MERGE CODE THAT IS RUN THROUGH THIS TOOL YET. +# +# WE ARE STILL DISCUSSING OUR DESIRED STYLE AND ITERATING ON IT. +# (12 Feb 2020) +# + # This script runs "clang-format" and "codetool" in sequence over each of # our source files, and replaces the original file only if it has changed. # diff --git a/scripts/maint/codetool.py b/scripts/maint/codetool.py index 6336e6843b..725712c0cc 100755 --- a/scripts/maint/codetool.py +++ b/scripts/maint/codetool.py @@ -2,6 +2,14 @@ # Copyright (c) 2020, The Tor Project, Inc. # See LICENSE for licensing information. +# +# DO NOT COMMIT OR MERGE CODE THAT IS RUN THROUGH THIS TOOL YET. +# +# WE ARE STILL DISCUSSING OUR DESIRED STYLE AND ITERATING ON IT, +# ALONG WITH THE TOOLS THAT ACHIEVE IT. +# (12 Feb 2020) +# + """ This program uses a set of plugable filters to inspect and transform our C code.