From 0b367f3386b0ec25f85716001690c95ae2e78c4d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 10:41:05 -0400 Subject: [PATCH 01/13] Add comments to annotate_ifdef_directives --- scripts/maint/annotate_ifdef_directives | 40 +++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index ca267a865e..15121652d7 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -2,21 +2,54 @@ # Copyright (c) 2017-2019, The Tor Project, Inc. # See LICENSE for licensing information +# This script iterates over a list of C files. For each file, it looks at the +# #if/#else C macros, and annotates them with comments explaining what they +# match. +# +# For example, it replaces this: +# +# #ifdef HAVE_OCELOT +# // 500 lines of ocelot code +# #endif +# +# with this: +# +# #ifdef HAVE_OCELOT +# // 500 lines of ocelot code +# #endif /* defined(HAVE_OCELOT) */ +# +# Note that only #else and #endif lines are annotated. Existing comments +# on those lines are removed. + import re +# Any block with fewer than this many lines does not need annotations. LINE_OBVIOUSNESS_LIMIT = 4 class Problem(Exception): pass def uncomment(s): + """ + Remove existing trailing comments from an #else or #endif line. + """ s = re.sub(r'//.*','',s) s = re.sub(r'/\*.*','',s) return s.strip() def translate(f_in, f_out): - whole_file = [] + """ + Read a file from f_in, and write its annotated version to f_out. + """ + # A stack listing our current if/else state. Each member of the stack + # is a list of directives. Each directive is a 3-tuple of + # (command, rest, lineno) + # where "command" is one of if/ifdef/ifndef/else/elif, and where + # "rest" is an expression in a format suitable for use with #if, and where + # lineno is the line number where the directive occurred. stack = [] + # the stack element corresponding to the top level of the file. + whole_file = [] cur_level = whole_file lineno = 0 for line in f_in: @@ -24,6 +57,7 @@ def translate(f_in, f_out): m = re.match(r'\s*#\s*(if|ifdef|ifndef|else|endif|elif)\b\s*(.*)', line) if not m: + # no directive, so we can just write it out. f_out.write(line) continue command,rest = m.groups() @@ -43,6 +77,8 @@ def translate(f_in, f_out): cur_level = new_level f_out.write(line) elif command in ("else", "elif"): + # We stay at the same level on the stack. If we have an #else, + # we comment it. if len(cur_level) == 0 or cur_level[-1][0] == 'else': raise Problem("Unexpected #%s on %d"% (command,lineno)) if (len(cur_level) == 1 and command == 'else' and @@ -52,6 +88,7 @@ def translate(f_in, f_out): f_out.write(line) cur_level.append((command, rest, lineno)) else: + # We pop one element on the stack, and comment an endif. assert command == 'endif' if len(stack) == 0: raise Problem("Unmatched #%s on %s"% (command,lineno)) @@ -71,4 +108,3 @@ for fn in sys.argv[1:]: with open(fn+"_OUT", 'w') as output_file: translate(open(fn, 'r'), output_file) os.rename(fn+"_OUT", fn) - From f36e743e5de92cecee81a64d2e78ce5ca3070f98 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 10:46:47 -0400 Subject: [PATCH 02/13] annotate_ifdef_directives: introduce a function to make commented lines No functional change in this commit. --- scripts/maint/annotate_ifdef_directives | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index 15121652d7..fcd96aeb38 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -29,6 +29,12 @@ LINE_OBVIOUSNESS_LIMIT = 4 class Problem(Exception): pass +def commented_line(fmt, argument): + """ + Return fmt%argument, for use as a commented line. + """ + return fmt % argument + def uncomment(s): """ Remove existing trailing comments from an #else or #endif line. @@ -83,7 +89,8 @@ def translate(f_in, f_out): raise Problem("Unexpected #%s on %d"% (command,lineno)) if (len(cur_level) == 1 and command == 'else' and lineno > cur_level[0][2] + LINE_OBVIOUSNESS_LIMIT): - f_out.write("#else /* !(%s) */\n"%cur_level[0][1]) + f_out.write(commented_line("#else /* !(%s) */\n", + cur_level[0][1])) else: f_out.write(line) cur_level.append((command, rest, lineno)) @@ -96,9 +103,11 @@ def translate(f_in, f_out): f_out.write(line) elif len(cur_level) == 1 or ( len(cur_level) == 2 and cur_level[1][0] == 'else'): - f_out.write("#endif /* %s */\n"%cur_level[0][1]) + f_out.write(commented_line("#endif /* %s */\n", + cur_level[0][1])) else: - f_out.write("#endif /* %s || ... */\n"%cur_level[0][1]) + f_out.write(commented_line("#endif /* %s || ... */\n", + cur_level[0][1])) cur_level = stack.pop() if len(stack) or cur_level != whole_file: raise Problem("Missing #endif") From 16890839d35e9ac270e32934a232b45de9e8544b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 10:51:05 -0400 Subject: [PATCH 03/13] annotate_ifdef_directives: obey an 80-column line-limit If we would add a comment making a line longer than 80 columns, instead truncate the variable portion of the comment until it just fits into 80 columns, with an ellipsis. --- scripts/maint/annotate_ifdef_directives | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index fcd96aeb38..b784ca71ba 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -26,14 +26,33 @@ import re # Any block with fewer than this many lines does not need annotations. LINE_OBVIOUSNESS_LIMIT = 4 +# Maximum line width. +LINE_WIDTH=80 + class Problem(Exception): pass -def commented_line(fmt, argument): +def commented_line(fmt, argument, maxwidth=LINE_WIDTH): """ - Return fmt%argument, for use as a commented line. + Return fmt%argument, for use as a commented line. If the line would + be longer than maxwidth, truncate argument. + + Requires that fmt%"..." will fit into maxwidth characters. """ - return fmt % argument + result = fmt % argument + if len(result) <= maxwidth: + return result + else: + # figure out how much we need to truncate by to fit the argument, + # plus an ellipsis. + ellipsis = "..." + result = fmt % (argument + ellipsis) + overrun = len(result) - maxwidth + truncated_argument = argument[:-overrun] + ellipsis + + result = fmt % truncated_argument + assert len(result) <= maxwidth + return result def uncomment(s): """ From 65e63e746120a03b54de51f1db148a7fa1aa27e2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 10:59:35 -0400 Subject: [PATCH 04/13] annotate_ifdef_directives: remove some cases of double negation This change should reduce the number of cases where we say "/* !(!defined(foo)) */" . This only does cases where we can use a regex to make sure that the simplification is guaranteed to be correct. Full boolean simplification would require this script to parse C, and nobody wants that. --- scripts/maint/annotate_ifdef_directives | 44 +++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index b784ca71ba..4463d83828 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -54,6 +54,46 @@ def commented_line(fmt, argument, maxwidth=LINE_WIDTH): assert len(result) <= maxwidth return result +def negate(expr): + """Return a negated version of expr; try to avoid double-negation. + + We usually wrap expressions in parentheses and add a "!". + >>> negate("A && B") + '!(A && B)' + + But if we recognize the expression as negated, we can restore it. + >>> negate(negate("A && B")) + 'A && B' + + The same applies for defined(FOO). + >>> negate("defined(FOO)") + '!defined(FOO)' + >>> negate(negate("defined(FOO)")) + 'defined(FOO)' + + Internal parentheses don't confuse us: + >>> negate("!(FOO) && !(BAR)") + '!(!(FOO) && !(BAR))' + + """ + expr = expr.strip() + # See whether we match !(...), with no intervening close-parens. + m = re.match(r'^!\s*\(([^\)]*)\)$', expr) + if m: + return m.group(1) + + + # See whether we match !?defined(...), with no intervening close-parens. + m = re.match(r'^(!?)\s*(defined\([^\)]*\))$', expr) + if m: + if m.group(1) == "!": + prefix = "" + else: + prefix = "!" + return prefix + m.group(2) + + return "!(%s)" % expr + def uncomment(s): """ Remove existing trailing comments from an #else or #endif line. @@ -108,8 +148,8 @@ def translate(f_in, f_out): raise Problem("Unexpected #%s on %d"% (command,lineno)) if (len(cur_level) == 1 and command == 'else' and lineno > cur_level[0][2] + LINE_OBVIOUSNESS_LIMIT): - f_out.write(commented_line("#else /* !(%s) */\n", - cur_level[0][1])) + f_out.write(commented_line("#else /* %s */\n", + negate(cur_level[0][1]))) else: f_out.write(line) cur_level.append((command, rest, lineno)) From 3283fd7e79913e25cd5e626d6bb3a12a05b2f3fc Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 11:06:54 -0400 Subject: [PATCH 05/13] Changes file for 31759 and 31779 --- changes/ticket31759 | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changes/ticket31759 diff --git a/changes/ticket31759 b/changes/ticket31759 new file mode 100644 index 0000000000..f7428f711c --- /dev/null +++ b/changes/ticket31759 @@ -0,0 +1,5 @@ + o Minor features (auto-formatting scripts): + - When annotating C macros, never generate a line that our check-spaces + script would reject. Closes ticket 31759. + - When annotating C macros, try to remove cases of double-negation. + Closes ticket 31779. From 194dbea24d1d05fa7b63b361b06054da4df011b9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 11:01:12 -0400 Subject: [PATCH 06/13] Run "make autostyle" with new "annotate_ifdef_directives" --- src/app/config/config.c | 6 +++--- src/lib/crypt_ops/compat_openssl.h | 2 +- src/lib/fs/dir.c | 2 +- src/lib/log/util_bug.h | 2 +- src/lib/math/fp.c | 2 +- src/lib/memarea/memarea.c | 2 +- src/lib/process/daemon.c | 2 +- src/lib/process/process.c | 2 +- src/lib/process/restrict.c | 2 +- src/lib/process/setuid.c | 2 +- src/lib/tls/tortls_openssl.c | 4 ++-- 11 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/app/config/config.c b/src/app/config/config.c index bdfa547fd7..6ee818ab0c 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -1455,7 +1455,7 @@ options_act_reversible(const or_options_t *old_options, char **msg) "on this OS/with this build."); goto rollback; } -#else /* !(!defined(HAVE_SYS_UN_H)) */ +#else /* defined(HAVE_SYS_UN_H) */ if (options->ControlSocketsGroupWritable && !options->ControlSocket) { *msg = tor_strdup("Setting ControlSocketGroupWritable without setting" "a ControlSocket makes no sense."); @@ -5101,7 +5101,7 @@ find_torrc_filename(config_line_t *cmd_arg, } else { fname = dflt ? tor_strdup(dflt) : NULL; } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ fname = dflt ? tor_strdup(dflt) : NULL; #endif /* !defined(_WIN32) */ } @@ -8425,7 +8425,7 @@ init_cookie_authentication(const char *fname, const char *header, log_warn(LD_FS,"Unable to make %s group-readable.", escaped(fname)); } } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ (void) group_readable; #endif /* !defined(_WIN32) */ diff --git a/src/lib/crypt_ops/compat_openssl.h b/src/lib/crypt_ops/compat_openssl.h index 9c10386c34..61ca51315f 100644 --- a/src/lib/crypt_ops/compat_openssl.h +++ b/src/lib/crypt_ops/compat_openssl.h @@ -45,7 +45,7 @@ ((st) == SSL3_ST_SW_SRVR_HELLO_B)) #define OSSL_HANDSHAKE_STATE int #define CONST_IF_OPENSSL_1_1_API -#else /* !(!defined(OPENSSL_1_1_API)) */ +#else /* defined(OPENSSL_1_1_API) */ #define STATE_IS_SW_SERVER_HELLO(st) \ ((st) == TLS_ST_SW_SRVR_HELLO) #define CONST_IF_OPENSSL_1_1_API const diff --git a/src/lib/fs/dir.c b/src/lib/fs/dir.c index 3c31e00d99..291f1bbf04 100644 --- a/src/lib/fs/dir.c +++ b/src/lib/fs/dir.c @@ -262,7 +262,7 @@ check_private_dir,(const char *dirname, cpd_check_t check, } } close(fd); -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ /* Win32 case: we can't open() a directory. */ (void)effective_user; diff --git a/src/lib/log/util_bug.h b/src/lib/log/util_bug.h index 546ae1e3ef..d7f01618e8 100644 --- a/src/lib/log/util_bug.h +++ b/src/lib/log/util_bug.h @@ -96,7 +96,7 @@ (void)(a); \ (void)(fmt); \ STMT_END -#else /* !(defined(TOR_UNIT_TESTS) && ... */ +#else /* !(defined(TOR_UNIT_TESTS) && defined(DISABLE_ASSERTS_IN_UNIT_TES... */ /** Like assert(3), but send assertion failures to the log as well as to * stderr. */ #define tor_assert(expr) tor_assertf(expr, NULL) diff --git a/src/lib/math/fp.c b/src/lib/math/fp.c index 616e4f15c0..49a2a6a2ca 100644 --- a/src/lib/math/fp.c +++ b/src/lib/math/fp.c @@ -75,7 +75,7 @@ clamp_double_to_int64(double number) */ #define PROBLEMATIC_FLOAT_CONVERSION_WARNING DISABLE_GCC_WARNING(float-conversion) -#endif /* defined(MINGW_ANY) && GCC_VERSION >= 409 */ +#endif /* (defined(MINGW_ANY)||defined(__FreeBSD__)) && GCC_VERSION >= 409 */ /* With clang 4.0 we apparently run into "double promotion" warnings here, diff --git a/src/lib/memarea/memarea.c b/src/lib/memarea/memarea.c index 84c73b0b95..0a88210906 100644 --- a/src/lib/memarea/memarea.c +++ b/src/lib/memarea/memarea.c @@ -315,7 +315,7 @@ memarea_assert_ok(memarea_t *area) } } -#else /* !(!defined(DISABLE_MEMORY_SENTINELS)) */ +#else /* defined(DISABLE_MEMORY_SENTINELS) */ struct memarea_t { smartlist_t *pieces; diff --git a/src/lib/process/daemon.c b/src/lib/process/daemon.c index 3b90bef671..ae34b5bcb8 100644 --- a/src/lib/process/daemon.c +++ b/src/lib/process/daemon.c @@ -165,7 +165,7 @@ finish_daemon(const char *desired_cwd) return 0; } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ /* defined(_WIN32) */ int start_daemon(void) diff --git a/src/lib/process/process.c b/src/lib/process/process.c index 631c7169f1..2194a603ff 100644 --- a/src/lib/process/process.c +++ b/src/lib/process/process.c @@ -513,7 +513,7 @@ process_get_unix_process(const process_t *process) tor_assert(process->unix_process); return process->unix_process; } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ /** Get the internal handle for Windows backend. */ process_win32_t * process_get_win32_process(const process_t *process) diff --git a/src/lib/process/restrict.c b/src/lib/process/restrict.c index 534b39d101..93d06de9a2 100644 --- a/src/lib/process/restrict.c +++ b/src/lib/process/restrict.c @@ -214,7 +214,7 @@ set_max_file_descriptors(rlim_t limit, int *max_out) return -1; } limit = MAX_CONNECTIONS; -#else /* !(!defined(HAVE_GETRLIMIT)) */ +#else /* defined(HAVE_GETRLIMIT) */ struct rlimit rlim; if (getrlimit(RLIMIT_NOFILE, &rlim) != 0) { diff --git a/src/lib/process/setuid.c b/src/lib/process/setuid.c index 6e8258f279..e132787943 100644 --- a/src/lib/process/setuid.c +++ b/src/lib/process/setuid.c @@ -376,7 +376,7 @@ switch_id(const char *user, const unsigned flags) #endif /* defined(__linux__) && defined(HAVE_SYS_PRCTL_H) && ... */ return 0; -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ (void)user; (void)flags; diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index 86f0ac42cc..58a7b20dec 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -657,7 +657,7 @@ tor_tls_context_new(crypto_pk_t *identity, unsigned int key_lifetime, if (r < 0) goto error; } -#else /* !(defined(SSL_CTX_set1_groups_list) || ...) */ +#else /* !(defined(SSL_CTX_set1_groups_list) || defined(HAVE_SSL_CTX_SET1... */ if (! is_client) { int nid; EC_KEY *ec_key; @@ -673,7 +673,7 @@ tor_tls_context_new(crypto_pk_t *identity, unsigned int key_lifetime, SSL_CTX_set_tmp_ecdh(result->ctx, ec_key); EC_KEY_free(ec_key); } -#endif /* defined(SSL_CTX_set1_groups_list) || ...) */ +#endif /* defined(SSL_CTX_set1_groups_list) || defined(HAVE_SSL_CTX_SET1_... */ SSL_CTX_set_verify(result->ctx, SSL_VERIFY_PEER, always_accept_verify_cb); /* let us realloc bufs that we're writing from */ From 21cc9d13f34d116b521c4415098ca1c9993bd198 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 25 Sep 2019 21:13:30 -0400 Subject: [PATCH 07/13] annotate_ifdef_directives: clarify situation with newlines Our line limit is 80 characters, assuming that there is a single terminating newline character that counts towards the limit. On Windows, this might go as high as 81 characters, if we count CRLF as two characters. --- scripts/maint/annotate_ifdef_directives | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index 4463d83828..b6bb147ce2 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -26,7 +26,10 @@ import re # Any block with fewer than this many lines does not need annotations. LINE_OBVIOUSNESS_LIMIT = 4 -# Maximum line width. +# Maximum line width. This includes a terminating newline character. +# +# (This is the maximum before encoding, so that if the the operating system +# uses multiple characers to encode newline, that's still okay.) LINE_WIDTH=80 class Problem(Exception): @@ -38,7 +41,10 @@ def commented_line(fmt, argument, maxwidth=LINE_WIDTH): be longer than maxwidth, truncate argument. Requires that fmt%"..." will fit into maxwidth characters. + + Requires that fmt ends with a newline. """ + assert fmt.endswith("\n") result = fmt % argument if len(result) <= maxwidth: return result From 195aa2f5f73e0cd9462afd4f21f3f0dac36bbc82 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 25 Sep 2019 21:27:17 -0400 Subject: [PATCH 08/13] annotate_ifdef_directives: generate paren-balanced expressions This algorithm is not fully general, but it strikes a balance between efficiency, simplicity, and correctness. --- scripts/maint/annotate_ifdef_directives | 46 ++++++++++++++++++++----- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index b6bb147ce2..f88dd4fdfe 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -35,7 +35,41 @@ LINE_WIDTH=80 class Problem(Exception): pass +def close_parens_needed(expr): + """Return the number of left-parentheses needed to make 'expr' + balanced. + """ + return expr.count("(") - expr.count(")") + +def truncate_expression(expr, new_width): + """Given a parenthesized C expression in 'expr', try to return a new + expression that is similar to 'expr', but no more than 'new_width' + characters long. + + Try to return an expression with balanced parentheses. + """ + if len(expr) <= new_width: + # The expression is already short enough. + return expr + + ellipsis = "..." + + # Start this at the minimum that we might truncate. + n_to_remove = len(expr) + len(ellipsis) - new_width + + # Try removing characters, one by one, until we get something where + # re-balancing the parentheses still fits within the limit. + while n_to_remove < len(expr): + truncated = expr[:-n_to_remove] + ellipsis + truncated += ")" * close_parens_needed(truncated) + if len(truncated) <= new_width: + return truncated + n_to_remove += 1 + + return ellipsis + def commented_line(fmt, argument, maxwidth=LINE_WIDTH): + """ Return fmt%argument, for use as a commented line. If the line would be longer than maxwidth, truncate argument. @@ -49,14 +83,10 @@ def commented_line(fmt, argument, maxwidth=LINE_WIDTH): if len(result) <= maxwidth: return result else: - # figure out how much we need to truncate by to fit the argument, - # plus an ellipsis. - ellipsis = "..." - result = fmt % (argument + ellipsis) - overrun = len(result) - maxwidth - truncated_argument = argument[:-overrun] + ellipsis - - result = fmt % truncated_argument + # How long can we let the argument be? Try filling in the + # format with an empty argument to find out. + max_arg_width = maxwidth - len(fmt % "") + result = fmt % truncate_expression(argument, max_arg_width) assert len(result) <= maxwidth return result From 6f0e697e4155ca567ddfd46a7f4e7c013287c42a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Sep 2019 10:03:28 -0400 Subject: [PATCH 09/13] Use Doctests to test the behavior of annotate_ifdef_directives. --- scripts/maint/annotate_ifdef_directives | 115 +++++++++++++++++++----- 1 file changed, 94 insertions(+), 21 deletions(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index f88dd4fdfe..514b5e58bb 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -2,24 +2,60 @@ # Copyright (c) 2017-2019, The Tor Project, Inc. # See LICENSE for licensing information -# This script iterates over a list of C files. For each file, it looks at the -# #if/#else C macros, and annotates them with comments explaining what they -# match. -# -# For example, it replaces this: -# -# #ifdef HAVE_OCELOT -# // 500 lines of ocelot code -# #endif -# -# with this: -# -# #ifdef HAVE_OCELOT -# // 500 lines of ocelot code -# #endif /* defined(HAVE_OCELOT) */ -# -# Note that only #else and #endif lines are annotated. Existing comments -# on those lines are removed. +r""" +This script iterates over a list of C files. For each file, it looks at the +#if/#else C macros, and annotates them with comments explaining what they +match. + +For example, it replaces this kind of input... + +>>> INPUT = ''' +... #ifdef HAVE_OCELOT +... C code here +... #if MIMSY == BOROGROVE +... block 1 +... block 1 +... block 1 +... block 1 +... #else +... block 2 +... block 2 +... block 2 +... block 2 +... #endif +... #endif +... ''' + +With this kind of output: +>>> EXPECTED_OUTPUT = ''' +... #ifdef HAVE_OCELOT +... C code here +... #if MIMSY == BOROGROVE +... block 1 +... block 1 +... block 1 +... block 1 +... #else /* !(MIMSY == BOROGROVE) */ +... block 2 +... block 2 +... block 2 +... block 2 +... #endif /* MIMSY == BOROGROVE */ +... #endif /* defined(HAVE_OCELOT) */ +... ''' + +Here's how to use it: +>>> import sys +>>> if sys.version_info.major < 3: from cStringIO import StringIO +>>> if sys.version_info.major >= 3: from io import StringIO + +>>> OUTPUT = StringIO() +>>> translate(StringIO(INPUT), OUTPUT) +>>> assert OUTPUT.getvalue() == EXPECTED_OUTPUT + +Note that only #else and #endif lines are annotated. Existing comments +on those lines are removed. +""" import re @@ -38,6 +74,17 @@ class Problem(Exception): def close_parens_needed(expr): """Return the number of left-parentheses needed to make 'expr' balanced. + + >>> close_parens_needed("1+2") + 0 + >>> close_parens_needed("(1 + 2)") + 0 + >>> close_parens_needed("(1 + 2") + 1 + >>> close_parens_needed("(1 + (2 *") + 2 + >>> close_parens_needed("(1 + (2 * 3) + (4") + 2 """ return expr.count("(") - expr.count(")") @@ -47,6 +94,17 @@ def truncate_expression(expr, new_width): characters long. Try to return an expression with balanced parentheses. + + >>> truncate_expression("1+2+3", 8) + '1+2+3' + >>> truncate_expression("1+2+3+4+5", 8) + '1+2+3...' + >>> truncate_expression("(1+2+3+4)", 8) + '(1+2...)' + >>> truncate_expression("(1+(2+3+4))", 8) + '(1+...)' + >>> truncate_expression("(((((((((", 8) + '((...))' """ if len(expr) <= new_width: # The expression is already short enough. @@ -69,14 +127,23 @@ def truncate_expression(expr, new_width): return ellipsis def commented_line(fmt, argument, maxwidth=LINE_WIDTH): - - """ + # (This is a raw docstring so that our doctests can use \.) + r""" Return fmt%argument, for use as a commented line. If the line would - be longer than maxwidth, truncate argument. + be longer than maxwidth, truncate argument but try to keep its + parentheses balanced. Requires that fmt%"..." will fit into maxwidth characters. Requires that fmt ends with a newline. + + >>> commented_line("/* %s */\n", "hello world", 32) + '/* hello world */\n' + >>> commented_line("/* %s */\n", "hello world", 15) + '/* hello... */\n' + >>> commented_line("#endif /* %s */\n", "((1+2) && defined(FOO))", 32) + '#endif /* ((1+2) && defi...) */\n' + """ assert fmt.endswith("\n") result = fmt % argument @@ -208,6 +275,12 @@ def translate(f_in, f_out): raise Problem("Missing #endif") import sys,os + +if sys.argv[1] == "--self-test": + import doctest + doctest.testmod() + sys.exit(0) + for fn in sys.argv[1:]: with open(fn+"_OUT", 'w') as output_file: translate(open(fn, 'r'), output_file) From d229399e77f17a8ad19a793fcc7252027c0d3758 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Sep 2019 15:36:20 -0400 Subject: [PATCH 10/13] annotate_ifdef_directives: Allow it to be imported as a module. --- scripts/maint/annotate_ifdef_directives | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives index 514b5e58bb..6ff9b8ec4c 100755 --- a/scripts/maint/annotate_ifdef_directives +++ b/scripts/maint/annotate_ifdef_directives @@ -274,14 +274,16 @@ def translate(f_in, f_out): if len(stack) or cur_level != whole_file: raise Problem("Missing #endif") -import sys,os +if __name__ == '__main__': -if sys.argv[1] == "--self-test": - import doctest - doctest.testmod() - sys.exit(0) + import sys,os -for fn in sys.argv[1:]: - with open(fn+"_OUT", 'w') as output_file: - translate(open(fn, 'r'), output_file) - os.rename(fn+"_OUT", fn) + if sys.argv[1] == "--self-test": + import doctest + doctest.testmod() + sys.exit(0) + + for fn in sys.argv[1:]: + with open(fn+"_OUT", 'w') as output_file: + translate(open(fn, 'r'), output_file) + os.rename(fn+"_OUT", fn) From f1e0665c934db49cc86936a17a3a1247db9e3337 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Sep 2019 15:43:40 -0400 Subject: [PATCH 11/13] Rename annotate_ifdef_directives to end with .py. This allows the python doctest module to process it correctly when invoked as: python -m doctest -v annotate_ifdef_directives.py --- Makefile.am | 2 +- .../{annotate_ifdef_directives => annotate_ifdef_directives.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename scripts/maint/{annotate_ifdef_directives => annotate_ifdef_directives.py} (100%) diff --git a/Makefile.am b/Makefile.am index 491b4c8f9f..e52b1f742a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -477,7 +477,7 @@ version: .PHONY: autostyle-ifdefs autostyle-ifdefs: - $(PYTHON) scripts/maint/annotate_ifdef_directives $(OWNED_TOR_C_FILES) + $(PYTHON) scripts/maint/annotate_ifdef_directives.py $(OWNED_TOR_C_FILES) .PHONY: autostyle-ifdefs autostyle-operators: diff --git a/scripts/maint/annotate_ifdef_directives b/scripts/maint/annotate_ifdef_directives.py similarity index 100% rename from scripts/maint/annotate_ifdef_directives rename to scripts/maint/annotate_ifdef_directives.py From 21c9f7c85e90f4d3ef539d41a36a24b2f26ad3d1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Sep 2019 19:57:41 -0400 Subject: [PATCH 12/13] Annotate_ifdef_directives: doctest for 80-column lines. --- scripts/maint/annotate_ifdef_directives.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/maint/annotate_ifdef_directives.py b/scripts/maint/annotate_ifdef_directives.py index 6ff9b8ec4c..b4326f9822 100755 --- a/scripts/maint/annotate_ifdef_directives.py +++ b/scripts/maint/annotate_ifdef_directives.py @@ -144,6 +144,18 @@ def commented_line(fmt, argument, maxwidth=LINE_WIDTH): >>> commented_line("#endif /* %s */\n", "((1+2) && defined(FOO))", 32) '#endif /* ((1+2) && defi...) */\n' + + The default line limit is 80 characters including the newline: + + >>> long_argument = "long " * 100 + >>> long_line = commented_line("#endif /* %s */\n", long_argument) + >>> len(long_line) + 80 + + >>> long_line[:40] + '#endif /* long long long long long long ' + >>> long_line[40:] + 'long long long long long long lon... */\n' """ assert fmt.endswith("\n") result = fmt % argument From fc1134e3e59df2ca473e787f70a57ebf659d78f2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 26 Sep 2019 20:30:41 -0400 Subject: [PATCH 13/13] annotate_ifdef_directives: test edge-case of 80-char line An 80-character line (79 characters if you don't count the newline) should not be truncated, and should not have a "..." insterted. --- scripts/maint/annotate_ifdef_directives.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/scripts/maint/annotate_ifdef_directives.py b/scripts/maint/annotate_ifdef_directives.py index b4326f9822..102128bfa0 100755 --- a/scripts/maint/annotate_ifdef_directives.py +++ b/scripts/maint/annotate_ifdef_directives.py @@ -156,6 +156,22 @@ def commented_line(fmt, argument, maxwidth=LINE_WIDTH): '#endif /* long long long long long long ' >>> long_line[40:] 'long long long long long long lon... */\n' + + If a line works out to being 80 characters naturally, it isn't truncated, + and no ellipsis is added. + + >>> medium_argument = "a"*66 + >>> medium_line = commented_line("#endif /* %s */\n", medium_argument) + >>> len(medium_line) + 80 + >>> "..." in medium_line + False + >>> medium_line[:40] + '#endif /* aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' + >>> medium_line[40:] + 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa */\n' + + """ assert fmt.endswith("\n") result = fmt % argument