[tor-commits] [tor/master] hacking is now markdown

commit 92a6c578d7038556af391de23081d314091dea32 Author: Nick Mathewson nickm@torproject.org Date: Thu Oct 29 10:30:27 2015 -0400

hacking is now markdown

Not good markdown, mind you. --- doc/HACKING/CodingStandards.md | 238 +++++++++++++++++++++++ doc/HACKING/CodingStandards.txt | 238 ----------------------- doc/HACKING/GettingStarted.md | 187 ++++++++++++++++++ doc/HACKING/GettingStarted.txt | 187 ------------------ doc/HACKING/HelpfulTools.md | 303 +++++++++++++++++++++++++++++ doc/HACKING/HelpfulTools.txt | 303 ----------------------------- doc/HACKING/HowToReview.md | 85 +++++++++ doc/HACKING/HowToReview.txt | 85 --------- doc/HACKING/README.1st | 60 ------ doc/HACKING/README.1st.md | 60 ++++++ doc/HACKING/ReleasingTor.md | 125 ++++++++++++ doc/HACKING/ReleasingTor.txt | 125 ------------ doc/HACKING/WritingTests.md | 403 +++++++++++++++++++++++++++++++++++++++ doc/HACKING/WritingTests.txt | 403 --------------------------------------- doc/include.am | 14 +- 15 files changed, 1408 insertions(+), 1408 deletions(-)

diff --git a/doc/HACKING/CodingStandards.md b/doc/HACKING/CodingStandards.md new file mode 100644 index 0000000..ff602bd --- /dev/null +++ b/doc/HACKING/CodingStandards.md @@ -0,0 +1,238 @@ +Coding conventions for Tor +-------------------------- + +tl;dr: + + * Run configure with '--enable-gcc-warnings' + * Run 'make check-spaces' to catch whitespace errors + * Document your functions + * Write unit tests + * Add a file in 'changes' for your branch. + +Patch checklist +~~~~~~~~~~~~~~~ + +If possible, send your patch as one of these (in descending order of +preference) + + - A git branch we can pull from + - Patches generated by git format-patch + - A unified diff + +Did you remember... + + - To build your code while configured with --enable-gcc-warnings? + - To run "make check-spaces" on your code? + - To run "make check-docs" to see whether all new options are on + the manpage? + - To write unit tests, as possible? + - To base your code on the appropriate branch? + - To include a file in the "changes" directory as appropriate? + +How we use Git branches +----------------------- + +Each main development series (like 0.2.1, 0.2.2, etc) has its main work +applied to a single branch. At most one series can be the development series +at a time; all other series are maintenance series that get bug-fixes only. +The development series is built in a git branch called "master"; the +maintenance series are built in branches called "maint-0.2.0", "maint-0.2.1", +and so on. We regularly merge the active maint branches forward. + +For all series except the development series, we also have a "release" branch +(as in "release-0.2.1"). The release series is based on the corresponding +maintenance series, except that it deliberately lags the maint series for +most of its patches, so that bugfix patches are not typically included in a +maintenance release until they've been tested for a while in a development +release. Occasionally, we'll merge an urgent bugfix into the release branch +before it gets merged into maint, but that's rare. + +If you're working on a bugfix for a bug that occurs in a particular version, +base your bugfix branch on the "maint" branch for the first supported series +that has that bug. (As of June 2013, we're supporting 0.2.3 and later.) If +you're working on a new feature, base it on the master branch. + + +How we log changes +------------------ + +When you do a commit that needs a ChangeLog entry, add a new file to +the "changes" toplevel subdirectory. It should have the format of a +one-entry changelog section from the current ChangeLog file, as in + + o Major bugfixes: + - Fix a potential buffer overflow. Fixes bug 99999; bugfix on + 0.3.1.4-beta. + +To write a changes file, first categorize the change. Some common categories +are: Minor bugfixes, Major bugfixes, Minor features, Major features, Code +simplifications and refactoring. Then say what the change does. If +it's a bugfix, mention what bug it fixes and when the bug was +introduced. To find out which Git tag the change was introduced in, +you can use "git describe --contains <sha1 of commit>". + +If at all possible, try to create this file in the same commit where you are +making the change. Please give it a distinctive name that no other branch will +use for the lifetime of your change. To verify the format of the changes file, +you can use "make check-changes". + +When we go to make a release, we will concatenate all the entries +in changes to make a draft changelog, and clear the directory. We'll +then edit the draft changelog into a nice readable format. + +What needs a changes file?:: + A not-exhaustive list: Anything that might change user-visible + behavior. Anything that changes internals, documentation, or the build + system enough that somebody could notice. Big or interesting code + rewrites. Anything about which somebody might plausibly wonder "when + did that happen, and/or why did we do that" 6 months down the line. + +Why use changes files instead of Git commit messages?:: + Git commit messages are written for developers, not users, and they + are nigh-impossible to revise after the fact. + +Why use changes files instead of entries in the ChangeLog?:: + Having every single commit touch the ChangeLog file tended to create + zillions of merge conflicts. + +Whitespace and C conformance +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Invoke "make check-spaces" from time to time, so it can tell you about +deviations from our C whitespace style. Generally, we use: + + - Unix-style line endings + - K&R-style indentation + - No space before newlines + - A blank line at the end of each file + - Never more than one blank line in a row + - Always spaces, never tabs + - No more than 79-columns per line. + - Two spaces per indent. + - A space between control keywords and their corresponding paren + "if (x)", "while (x)", and "switch (x)", never "if(x)", "while(x)", or + "switch(x)". + - A space between anything and an open brace. + - No space between a function name and an opening paren. "puts(x)", not + "puts (x)". + - Function declarations at the start of the line. + +We try hard to build without warnings everywhere. In particular, if you're +using gcc, you should invoke the configure script with the option +"--enable-gcc-warnings". This will give a bunch of extra warning flags to +the compiler, and help us find divergences from our preferred C style. + +Functions to use; functions not to use +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +We have some wrapper functions like tor_malloc, tor_free, tor_strdup, and +tor_gettimeofday; use them instead of their generic equivalents. (They +always succeed or exit.) + +You can get a full list of the compatibility functions that Tor provides by +looking through src/common/util*.h and src/common/compat*.h. You can see the +available containers in src/common/containers*.h. You should probably +familiarize yourself with these modules before you write too much code, or +else you'll wind up reinventing the wheel. + +Use 'INLINE' instead of 'inline' -- it's a vestige of an old hack to make +sure that we worked on MSVC6. + +We don't use strcat or strcpy or sprintf of any of those notoriously broken +old C functions. Use strlcat, strlcpy, or tor_snprintf/tor_asprintf instead. + +We don't call memcmp() directly. Use fast_memeq(), fast_memneq(), +tor_memeq(), or tor_memneq() for most purposes. + +Functions not to write +~~~~~~~~~~~~~~~~~~~~~~ + +Try to never hand-write new code to parse or generate binary +formats. Instead, use trunnel if at all possible. See + https://gitweb.torproject.org/trunnel.git/tree +for more information about trunnel. + +For information on adding new trunnel code to Tor, see src/trunnel/README + + +Calling and naming conventions +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Whenever possible, functions should return -1 on error and 0 on success. + +For multi-word identifiers, use lowercase words combined with +underscores. (e.g., "multi_word_identifier"). Use ALL_CAPS for macros and +constants. + +Typenames should end with "_t". + +Function names should be prefixed with a module name or object name. (In +general, code to manipulate an object should be a module with the same name +as the object, so it's hard to tell which convention is used.) + +Functions that do things should have imperative-verb names +(e.g. buffer_clear, buffer_resize); functions that return booleans should +have predicate names (e.g. buffer_is_empty, buffer_needs_resizing). + +If you find that you have four or more possible return code values, it's +probably time to create an enum. If you find that you are passing three or +more flags to a function, it's probably time to create a flags argument that +takes a bitfield. + +What To Optimize +~~~~~~~~~~~~~~~~ + +Don't optimize anything if it's not in the critical path. Right now, the +critical path seems to be AES, logging, and the network itself. Feel free to +do your own profiling to determine otherwise. + +Log conventions +~~~~~~~~~~~~~~~ + +https://www.torproject.org/docs/faq#LogLevel + +No error or warning messages should be expected during normal OR or OP +operation. + +If a library function is currently called such that failure always means ERR, +then the library function should log WARN and let the caller log ERR. + +Every message of severity INFO or higher should either (A) be intelligible +to end-users who don't know the Tor source; or (B) somehow inform the +end-users that they aren't expected to understand the message (perhaps +with a string like "internal error"). Option (A) is to be preferred to +option (B). + + + +Doxygen comment conventions +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Say what functions do as a series of one or more imperative sentences, as +though you were telling somebody how to be the function. In other words, DO +NOT say: + + /** The strtol function parses a number. + * + * nptr -- the string to parse. It can include whitespace. + * endptr -- a string pointer to hold the first thing that is not part + * of the number, if present. + * base -- the numeric base. + * returns: the resulting number. + */ + long strtol(const char *nptr, char **nptr, int base); + +Instead, please DO say: + + /** Parse a number in radix <b>base</b> from the string <b>nptr</b>, + * and return the result. Skip all leading whitespace. If + * <b>endptr</b> is not NULL, set *<b>endptr</b> to the first character + * after the number parsed. + **/ + long strtol(const char *nptr, char **nptr, int base); + +Doxygen comments are the contract in our abstraction-by-contract world: if +the functions that call your function rely on it doing something, then your +function should mention that it does that something in the documentation. If +you rely on a function doing something beyond what is in its documentation, +then you should watch out, or it might do something else later. diff --git a/doc/HACKING/CodingStandards.txt b/doc/HACKING/CodingStandards.txt deleted file mode 100644 index ff602bd..0000000 --- a/doc/HACKING/CodingStandards.txt +++ /dev/null @@ -1,238 +0,0 @@ -Coding conventions for Tor --------------------------- - -tl;dr: - - * Run configure with '--enable-gcc-warnings' - * Run 'make check-spaces' to catch whitespace errors - * Document your functions - * Write unit tests - * Add a file in 'changes' for your branch. - -Patch checklist -~~~~~~~~~~~~~~~ - -If possible, send your patch as one of these (in descending order of -preference) - - - A git branch we can pull from - - Patches generated by git format-patch - - A unified diff - -Did you remember... - - - To build your code while configured with --enable-gcc-warnings? - - To run "make check-spaces" on your code? - - To run "make check-docs" to see whether all new options are on - the manpage? - - To write unit tests, as possible? - - To base your code on the appropriate branch? - - To include a file in the "changes" directory as appropriate? - -How we use Git branches ------------------------ - -Each main development series (like 0.2.1, 0.2.2, etc) has its main work -applied to a single branch. At most one series can be the development series -at a time; all other series are maintenance series that get bug-fixes only. -The development series is built in a git branch called "master"; the -maintenance series are built in branches called "maint-0.2.0", "maint-0.2.1", -and so on. We regularly merge the active maint branches forward. - -For all series except the development series, we also have a "release" branch -(as in "release-0.2.1"). The release series is based on the corresponding -maintenance series, except that it deliberately lags the maint series for -most of its patches, so that bugfix patches are not typically included in a -maintenance release until they've been tested for a while in a development -release. Occasionally, we'll merge an urgent bugfix into the release branch -before it gets merged into maint, but that's rare. - -If you're working on a bugfix for a bug that occurs in a particular version, -base your bugfix branch on the "maint" branch for the first supported series -that has that bug. (As of June 2013, we're supporting 0.2.3 and later.) If -you're working on a new feature, base it on the master branch. - - -How we log changes ------------------- - -When you do a commit that needs a ChangeLog entry, add a new file to -the "changes" toplevel subdirectory. It should have the format of a -one-entry changelog section from the current ChangeLog file, as in - - o Major bugfixes: - - Fix a potential buffer overflow. Fixes bug 99999; bugfix on - 0.3.1.4-beta. - -To write a changes file, first categorize the change. Some common categories -are: Minor bugfixes, Major bugfixes, Minor features, Major features, Code -simplifications and refactoring. Then say what the change does. If -it's a bugfix, mention what bug it fixes and when the bug was -introduced. To find out which Git tag the change was introduced in, -you can use "git describe --contains <sha1 of commit>". - -If at all possible, try to create this file in the same commit where you are -making the change. Please give it a distinctive name that no other branch will -use for the lifetime of your change. To verify the format of the changes file, -you can use "make check-changes". - -When we go to make a release, we will concatenate all the entries -in changes to make a draft changelog, and clear the directory. We'll -then edit the draft changelog into a nice readable format. - -What needs a changes file?:: - A not-exhaustive list: Anything that might change user-visible - behavior. Anything that changes internals, documentation, or the build - system enough that somebody could notice. Big or interesting code - rewrites. Anything about which somebody might plausibly wonder "when - did that happen, and/or why did we do that" 6 months down the line. - -Why use changes files instead of Git commit messages?:: - Git commit messages are written for developers, not users, and they - are nigh-impossible to revise after the fact. - -Why use changes files instead of entries in the ChangeLog?:: - Having every single commit touch the ChangeLog file tended to create - zillions of merge conflicts. - -Whitespace and C conformance -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Invoke "make check-spaces" from time to time, so it can tell you about -deviations from our C whitespace style. Generally, we use: - - - Unix-style line endings - - K&R-style indentation - - No space before newlines - - A blank line at the end of each file - - Never more than one blank line in a row - - Always spaces, never tabs - - No more than 79-columns per line. - - Two spaces per indent. - - A space between control keywords and their corresponding paren - "if (x)", "while (x)", and "switch (x)", never "if(x)", "while(x)", or - "switch(x)". - - A space between anything and an open brace. - - No space between a function name and an opening paren. "puts(x)", not - "puts (x)". - - Function declarations at the start of the line. - -We try hard to build without warnings everywhere. In particular, if you're -using gcc, you should invoke the configure script with the option -"--enable-gcc-warnings". This will give a bunch of extra warning flags to -the compiler, and help us find divergences from our preferred C style. - -Functions to use; functions not to use -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -We have some wrapper functions like tor_malloc, tor_free, tor_strdup, and -tor_gettimeofday; use them instead of their generic equivalents. (They -always succeed or exit.) - -You can get a full list of the compatibility functions that Tor provides by -looking through src/common/util*.h and src/common/compat*.h. You can see the -available containers in src/common/containers*.h. You should probably -familiarize yourself with these modules before you write too much code, or -else you'll wind up reinventing the wheel. - -Use 'INLINE' instead of 'inline' -- it's a vestige of an old hack to make -sure that we worked on MSVC6. - -We don't use strcat or strcpy or sprintf of any of those notoriously broken -old C functions. Use strlcat, strlcpy, or tor_snprintf/tor_asprintf instead. - -We don't call memcmp() directly. Use fast_memeq(), fast_memneq(), -tor_memeq(), or tor_memneq() for most purposes. - -Functions not to write -~~~~~~~~~~~~~~~~~~~~~~ - -Try to never hand-write new code to parse or generate binary -formats. Instead, use trunnel if at all possible. See - https://gitweb.torproject.org/trunnel.git/tree -for more information about trunnel. - -For information on adding new trunnel code to Tor, see src/trunnel/README - - -Calling and naming conventions -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Whenever possible, functions should return -1 on error and 0 on success. - -For multi-word identifiers, use lowercase words combined with -underscores. (e.g., "multi_word_identifier"). Use ALL_CAPS for macros and -constants. - -Typenames should end with "_t". - -Function names should be prefixed with a module name or object name. (In -general, code to manipulate an object should be a module with the same name -as the object, so it's hard to tell which convention is used.) - -Functions that do things should have imperative-verb names -(e.g. buffer_clear, buffer_resize); functions that return booleans should -have predicate names (e.g. buffer_is_empty, buffer_needs_resizing). - -If you find that you have four or more possible return code values, it's -probably time to create an enum. If you find that you are passing three or -more flags to a function, it's probably time to create a flags argument that -takes a bitfield. - -What To Optimize -~~~~~~~~~~~~~~~~ - -Don't optimize anything if it's not in the critical path. Right now, the -critical path seems to be AES, logging, and the network itself. Feel free to -do your own profiling to determine otherwise. - -Log conventions -~~~~~~~~~~~~~~~ - -https://www.torproject.org/docs/faq#LogLevel - -No error or warning messages should be expected during normal OR or OP -operation. - -If a library function is currently called such that failure always means ERR, -then the library function should log WARN and let the caller log ERR. - -Every message of severity INFO or higher should either (A) be intelligible -to end-users who don't know the Tor source; or (B) somehow inform the -end-users that they aren't expected to understand the message (perhaps -with a string like "internal error"). Option (A) is to be preferred to -option (B). - - - -Doxygen comment conventions -^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Say what functions do as a series of one or more imperative sentences, as -though you were telling somebody how to be the function. In other words, DO -NOT say: - - /** The strtol function parses a number. - * - * nptr -- the string to parse. It can include whitespace. - * endptr -- a string pointer to hold the first thing that is not part - * of the number, if present. - * base -- the numeric base. - * returns: the resulting number. - */ - long strtol(const char *nptr, char **nptr, int base); - -Instead, please DO say: - - /** Parse a number in radix <b>base</b> from the string <b>nptr</b>, - * and return the result. Skip all leading whitespace. If - * <b>endptr</b> is not NULL, set *<b>endptr</b> to the first character - * after the number parsed. - **/ - long strtol(const char *nptr, char **nptr, int base); - -Doxygen comments are the contract in our abstraction-by-contract world: if -the functions that call your function rely on it doing something, then your -function should mention that it does that something in the documentation. If -you rely on a function doing something beyond what is in its documentation, -then you should watch out, or it might do something else later. diff --git a/doc/HACKING/GettingStarted.md b/doc/HACKING/GettingStarted.md new file mode 100644 index 0000000..ee024fe --- /dev/null +++ b/doc/HACKING/GettingStarted.md @@ -0,0 +1,187 @@ + +Getting started in Tor development +================================== + +Congratulations! You've found this file, and you're reading it! This +means that you might be interested in getting started in developing Tor. + +(This guide is just about Tor itself--the small network program at the +heart of the Tor network--and not about all the other programs in the +whole Tor ecosystem.) + + +If you are looking for a more bare-bones, less user-friendly information +dump of important information, you might like reading doc/HACKING +instead. You should probably read it before you write your first patch. + + +Required background +------------------- + +First, I'm going to assume that you can build Tor from source, and that +you know enough of the C language to read and write it. (See the README +file that comes with the Tor source for more information on building it, +and any high-quality guide to C for information on programming.) + +I'm also going to assume that you know a little bit about how to use +Git, or that you're able to follow one of the several excellent guides +at http://git-scm.org to learn. + +Most Tor developers develop using some Unix-based system, such as Linux, +BSD, or OSX. It's okay to develop on Windows if you want, but you're +going to have a more difficult time. + + +Getting your first patch into Tor +--------------------------------- + +Once you've reached this point, here's what you need to know. + + 1) Get the source. + + We keep our source under version control in Git. To get the latest + version, run + git clone https://git.torproject.org/git/tor + + This will give you a checkout of the master branch. If you're + going to fix a bug that appears in a stable version, check out the + appropriate "maint" branch, as in: + + git checkout maint-0.2.7 + + 2) Find your way around the source + + Our overall code structure is explained in the "torguts" documents, + currently at + git clone https://git.torproject.org/user/nickm/torguts.git + + Find a part of the code that looks interesting to you, and start + looking around it to see how it fits together! + + We do some unusual things in our codebase. Our testing-related + practices and kludges are explained in doc/WritingTests.txt. + + If you see something that doesn't make sense, we love to get + questions! + + 3) Find something cool to hack on. + + You may already have a good idea of what you'd like to work on, or + you might be looking for a way to contribute. + + Many people have gotten started by looking for an area where they + personally felt Tor was underperforming, and investigating ways to + fix it. If you're looking for ideas, you can head to our bug + tracker at trac.torproject.org and look for tickets that have + received the "easy" tag: these are ones that developers think would + be pretty simple for a new person to work on. For a bigger + challenge, you might want to look for tickets with the "lorax" + keyword: these are tickets that the developers think might be a + good idea to build, but which we have no time to work on any time + soon. + + Or you might find another open ticket that piques your + interest. It's all fine! + + For your first patch, it is probably NOT a good idea to make + something huge or invasive. In particular, you should probably + avoid: + * Major changes spread across many parts of the codebase. + * Major changes to programming practice or coding style. + * Huge new features or protocol changes. + + 4) Meet the developers! + + We discuss stuff on the tor-dev mailing list and on the #tor-dev + IRC channel on OFTC. We're generally friendly and approachable, + and we like to talk about how Tor fits together. If we have ideas + about how something should be implemented, we'll be happy to share + them. + + We currently have a patch workshop at least once a week, where + people share patches they've made and discuss how to make them + better. The time might change in the future, but generally, + there's no bad time to talk, and ask us about patch ideas. + + 5) Do you need to write a design proposal? + + If your idea is very large, or it will require a change to Tor's + protocols, there needs to be a written design proposal before it + can be merged. (We use this process to manage changes in the + protocols.) To write one, see the instructions at + https://gitweb.torproject.org/torspec.git/tree/proposals/001-process.txt + . If you'd like help writing a proposal, just ask! We're happy to + help out with good ideas. + + You might also like to look around the rest of that directory, to + see more about open and past proposed changes to Tor's behavior. + + 6) Writing your patch + + As you write your code, you'll probably want it to fit in with the + standards of the rest of the Tor codebase so it will be easy for us + to review and merge. You can learn our coding standards in + doc/HACKING. + + If your patch is large and/or is divided into multiple logical + components, remember to divide it into a series of Git commits. A + series of small changes is much easier to review than one big lump. + + 7) Testing your patch + + We prefer that all new or modified code have unit tests for it to + ensure that it runs correctly. Also, all code should actually be + _run_ by somebody, to make sure it works. + + See doc/WritingTests.txt for more information on how we test things + in Tor. If you'd like any help writing tests, just ask! We're + glad to help out. + + 8) Submitting your patch + + We review patches through tickets on our bugtracker at + trac.torproject.org. You can either upload your patches there, or + put them at a public git repository somewhere we can fetch them + (like github or bitbucket) and then paste a link on the appropriate + trac ticket. + + Once your patches are available, write a short explanation of what + you've done on trac, and then change the status of the ticket to + needs_review. + + 9) Review, Revision, and Merge + + With any luck, somebody will review your patch soon! If not, you + can ask on the IRC channel; sometimes we get really busy and take + longer than we should. But don't let us slow you down: you're the + one who's offering help here, and we should respect your time and + contributions. + + When your patch is reviewed, one of these things will happen: + + * The reviewer will say "looks good to me" and your + patch will get merged right into Tor. [Assuming we're not + in the middle of a code-freeze window. If the codebase is + frozen, your patch will go into the next release series.] + + * OR the reviewer will say "looks good, just needs some small + changes!" And then the reviewer will make those changes, + and merge the modified patch into Tor. + + * OR the reviewer will say "Here are some questions and + comments," followed by a bunch of stuff that the reviewer + thinks should change in your code, or questions that the + reviewer has. + + At this point, you might want to make the requested changes + yourself, and comment on the trac ticket once you have done + so. Or if you disagree with any of the comments, you should + say so! And if you won't have time to make some of the + changes, you should say that too, so that other developers + will be able to pick up the unfinished portion + + Congratulations! You have now written your first patch, and gotten + it integrated into mainline Tor. + + + diff --git a/doc/HACKING/GettingStarted.txt b/doc/HACKING/GettingStarted.txt deleted file mode 100644 index ee024fe..0000000 --- a/doc/HACKING/GettingStarted.txt +++ /dev/null @@ -1,187 +0,0 @@ - -Getting started in Tor development -================================== - -Congratulations! You've found this file, and you're reading it! This -means that you might be interested in getting started in developing Tor. - -(This guide is just about Tor itself--the small network program at the -heart of the Tor network--and not about all the other programs in the -whole Tor ecosystem.) - - -If you are looking for a more bare-bones, less user-friendly information -dump of important information, you might like reading doc/HACKING -instead. You should probably read it before you write your first patch. - - -Required background -------------------- - -First, I'm going to assume that you can build Tor from source, and that -you know enough of the C language to read and write it. (See the README -file that comes with the Tor source for more information on building it, -and any high-quality guide to C for information on programming.) - -I'm also going to assume that you know a little bit about how to use -Git, or that you're able to follow one of the several excellent guides -at http://git-scm.org to learn. - -Most Tor developers develop using some Unix-based system, such as Linux, -BSD, or OSX. It's okay to develop on Windows if you want, but you're -going to have a more difficult time. - - -Getting your first patch into Tor ---------------------------------- - -Once you've reached this point, here's what you need to know. - - 1) Get the source. - - We keep our source under version control in Git. To get the latest - version, run - git clone https://git.torproject.org/git/tor - - This will give you a checkout of the master branch. If you're - going to fix a bug that appears in a stable version, check out the - appropriate "maint" branch, as in: - - git checkout maint-0.2.7 - - 2) Find your way around the source - - Our overall code structure is explained in the "torguts" documents, - currently at - git clone https://git.torproject.org/user/nickm/torguts.git - - Find a part of the code that looks interesting to you, and start - looking around it to see how it fits together! - - We do some unusual things in our codebase. Our testing-related - practices and kludges are explained in doc/WritingTests.txt. - - If you see something that doesn't make sense, we love to get - questions! - - 3) Find something cool to hack on. - - You may already have a good idea of what you'd like to work on, or - you might be looking for a way to contribute. - - Many people have gotten started by looking for an area where they - personally felt Tor was underperforming, and investigating ways to - fix it. If you're looking for ideas, you can head to our bug - tracker at trac.torproject.org and look for tickets that have - received the "easy" tag: these are ones that developers think would - be pretty simple for a new person to work on. For a bigger - challenge, you might want to look for tickets with the "lorax" - keyword: these are tickets that the developers think might be a - good idea to build, but which we have no time to work on any time - soon. - - Or you might find another open ticket that piques your - interest. It's all fine! - - For your first patch, it is probably NOT a good idea to make - something huge or invasive. In particular, you should probably - avoid: - * Major changes spread across many parts of the codebase. - * Major changes to programming practice or coding style. - * Huge new features or protocol changes. - - 4) Meet the developers! - - We discuss stuff on the tor-dev mailing list and on the #tor-dev - IRC channel on OFTC. We're generally friendly and approachable, - and we like to talk about how Tor fits together. If we have ideas - about how something should be implemented, we'll be happy to share - them. - - We currently have a patch workshop at least once a week, where - people share patches they've made and discuss how to make them - better. The time might change in the future, but generally, - there's no bad time to talk, and ask us about patch ideas. - - 5) Do you need to write a design proposal? - - If your idea is very large, or it will require a change to Tor's - protocols, there needs to be a written design proposal before it - can be merged. (We use this process to manage changes in the - protocols.) To write one, see the instructions at - https://gitweb.torproject.org/torspec.git/tree/proposals/001-process.txt - . If you'd like help writing a proposal, just ask! We're happy to - help out with good ideas. - - You might also like to look around the rest of that directory, to - see more about open and past proposed changes to Tor's behavior. - - 6) Writing your patch - - As you write your code, you'll probably want it to fit in with the - standards of the rest of the Tor codebase so it will be easy for us - to review and merge. You can learn our coding standards in - doc/HACKING. - - If your patch is large and/or is divided into multiple logical - components, remember to divide it into a series of Git commits. A - series of small changes is much easier to review than one big lump. - - 7) Testing your patch - - We prefer that all new or modified code have unit tests for it to - ensure that it runs correctly. Also, all code should actually be - _run_ by somebody, to make sure it works. - - See doc/WritingTests.txt for more information on how we test things - in Tor. If you'd like any help writing tests, just ask! We're - glad to help out. - - 8) Submitting your patch - - We review patches through tickets on our bugtracker at - trac.torproject.org. You can either upload your patches there, or - put them at a public git repository somewhere we can fetch them - (like github or bitbucket) and then paste a link on the appropriate - trac ticket. - - Once your patches are available, write a short explanation of what - you've done on trac, and then change the status of the ticket to - needs_review. - - 9) Review, Revision, and Merge - - With any luck, somebody will review your patch soon! If not, you - can ask on the IRC channel; sometimes we get really busy and take - longer than we should. But don't let us slow you down: you're the - one who's offering help here, and we should respect your time and - contributions. - - When your patch is reviewed, one of these things will happen: - - * The reviewer will say "looks good to me" and your - patch will get merged right into Tor. [Assuming we're not - in the middle of a code-freeze window. If the codebase is - frozen, your patch will go into the next release series.] - - * OR the reviewer will say "looks good, just needs some small - changes!" And then the reviewer will make those changes, - and merge the modified patch into Tor. - - * OR the reviewer will say "Here are some questions and - comments," followed by a bunch of stuff that the reviewer - thinks should change in your code, or questions that the - reviewer has. - - At this point, you might want to make the requested changes - yourself, and comment on the trac ticket once you have done - so. Or if you disagree with any of the comments, you should - say so! And if you won't have time to make some of the - changes, you should say that too, so that other developers - will be able to pick up the unfinished portion - - Congratulations! You have now written your first patch, and gotten - it integrated into mainline Tor. - - - diff --git a/doc/HACKING/HelpfulTools.md b/doc/HACKING/HelpfulTools.md new file mode 100644 index 0000000..cf74063 --- /dev/null +++ b/doc/HACKING/HelpfulTools.md @@ -0,0 +1,303 @@ +Useful tools +------------ + +These aren't strictly necessary for hacking on Tor, but they can help track +down bugs. + +Jenkins +~~~~~~~ + +https://jenkins.torproject.org + +Dmalloc +~~~~~~~ + +The dmalloc library will keep track of memory allocation, so you can find out +if we're leaking memory, doing any double-frees, or so on. + + dmalloc -l ~/dmalloc.log + (run the commands it tells you) + ./configure --with-dmalloc + +Valgrind +~~~~~~~~ + +valgrind --leak-check=yes --error-limit=no --show-reachable=yes src/or/tor + +(Note that if you get a zillion openssl warnings, you will also need to +pass --undef-value-errors=no to valgrind, or rebuild your openssl +with -DPURIFY.) + +Coverity +~~~~~~~~ + +Nick regularly runs the coverity static analyzer on the Tor codebase. + +The preprocessor define __COVERITY__ is used to work around instances +where coverity picks up behavior that we wish to permit. + +clang Static Analyzer +~~~~~~~~~~~~~~~~~~~~~ + +The clang static analyzer can be run on the Tor codebase using Xcode (WIP) +or a command-line build. + +The preprocessor define __clang_analyzer__ is used to work around instances +where clang picks up behavior that we wish to permit. + +clang Runtime Sanitizers +~~~~~~~~~~~~~~~~~~~~~~~~ + +To build the Tor codebase with the clang Address and Undefined Behavior +sanitizers, see the file contrib/clang/sanitize_blacklist.txt. + +Preprocessor workarounds for instances where clang picks up behavior that +we wish to permit are also documented in the blacklist file. + +Running lcov for unit test coverage +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Lcov is a utility that generates pretty HTML reports of test code coverage. +To generate such a report: + +----- + ./configure --enable-coverage + make + make coverage-html + $BROWSER ./coverage_html/index.html +----- + +This will run the tor unit test suite `./src/test/test` and generate the HTML +coverage code report under the directory ./coverage_html/. To change the +output directory, use `make coverage-html HTML_COVER_DIR=./funky_new_cov_dir`. + +Coverage diffs using lcov are not currently implemented, but are being +investigated (as of July 2014). + +Running the unit tests +~~~~~~~~~~~~~~~~~~~~~~ + +To quickly run all the tests distributed with Tor: +----- + make check +----- + +To run the fast unit tests only: +----- + make test +----- + +To selectively run just some tests (the following can be combined +arbitrarily): +----- + ./src/test/test <name_of_test> [<name of test 2>] ... + ./src/test/test <prefix_of_name_of_test>.. [<prefix_of_name_of_test2>..] ... + ./src/test/test :<name_of_excluded_test> [:<name_of_excluded_test2]... +----- + +To run all tests, including those based on Stem or Chutney: +----- + make test-full +----- + +To run all tests, including those based on Stem or Chutney that require a +working connection to the internet: +----- + make test-full-online +----- + +Running gcov for unit test coverage +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +----- + ./configure --enable-coverage + make + make check + # or--- make test-full ? make test-full-online? + mkdir coverage-output + ./scripts/test/coverage coverage-output +----- + +(On OSX, you'll need to start with "--enable-coverage CC=clang".) + +Then, look at the .gcov files in coverage-output. '-' before a line means +that the compiler generated no code for that line. '######' means that the +line was never reached. Lines with numbers were called that number of times. + +If that doesn't work: + * Try configuring Tor with --disable-gcc-hardening + * You might need to run 'make clean' after you run './configure'. + +If you make changes to Tor and want to get another set of coverage results, +you can run "make reset-gcov" to clear the intermediary gcov output. + +If you have two different "coverage-output" directories, and you want to see +a meaningful diff between them, you can run: + +----- + ./scripts/test/cov-diff coverage-output1 coverage-output2 | less +----- + +In this diff, any lines that were visited at least once will have coverage +"1". This lets you inspect what you (probably) really want to know: which +untested lines were changed? Are there any new untested lines? + +Running integration tests +~~~~~~~~~~~~~~~~~~~~~~~~~ + +We have the beginnings of a set of scripts to run integration tests using +Chutney. To try them, set CHUTNEY_PATH to your chutney source directory, and +run "make test-network". + +We also have scripts to run integration tests using Stem. To try them, set +STEM_SOURCE_DIR to your Stem source directory, and run "test-stem". + +Profiling Tor with oprofile +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The oprofile tool runs (on Linux only!) to tell you what functions Tor is +spending its CPU time in, so we can identify performance bottlenecks. + +Here are some basic instructions + + - Build tor with debugging symbols (you probably already have, unless + you messed with CFLAGS during the build process). + - Build all the libraries you care about with debugging symbols + (probably you only care about libssl, maybe zlib and Libevent). + - Copy this tor to a new directory + - Copy all the libraries it uses to that dir too (ldd ./tor will + tell you) + - Set LD_LIBRARY_PATH to include that dir. ldd ./tor should now + show you it's using the libs in that dir + - Run that tor + - Reset oprofiles counters/start it + * "opcontrol --reset; opcontrol --start", if Nick remembers right. + - After a while, have it dump the stats on tor and all the libs + in that dir you created. + * "opcontrol --dump;" + * "opreport -l that_dir/*" + - Profit + +Generating and analyzing a callgraph +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +1. Run ./scripts/maint/generate_callgraph.sh . This will generate a + bunch of files in a new ./callgraph directory. + +2. Run ./scripts/maint/analyze_callgraph.py callgraph/src/*/* . This + will do a lot of graph operations and then dump out a new + "callgraph.pkl" file, containing data in Python's "pickle" format. + +3. Run ./scripts/maint/display_callgraph.py . It will display: + - the number of functions reachable from each function. + - all strongly-connnected components in the Tor callgraph + - the largest bottlenecks in the largest SCC in the Tor callgraph. + +Note that currently the callgraph generator can't detect calls that pass +through function pointers. + +Getting emacs to edit Tor source properly +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Nick likes to put the following snippet in his .emacs file: + +----- + (add-hook 'c-mode-hook + (lambda () + (font-lock-mode 1) + (set-variable 'show-trailing-whitespace t) + + (let ((fname (expand-file-name (buffer-file-name)))) + (cond + ((string-match "^/home/nickm/src/libevent" fname) + (set-variable 'indent-tabs-mode t) + (set-variable 'c-basic-offset 4) + (set-variable 'tab-width 4)) + ((string-match "^/home/nickm/src/tor" fname) + (set-variable 'indent-tabs-mode nil) + (set-variable 'c-basic-offset 2)) + ((string-match "^/home/nickm/src/openssl" fname) + (set-variable 'indent-tabs-mode t) + (set-variable 'c-basic-offset 8) + (set-variable 'tab-width 8)) + )))) +----- + +You'll note that it defaults to showing all trailing whitespace. The "cond" +test detects whether the file is one of a few C free software projects that I +often edit, and sets up the indentation level and tab preferences to match +what they want. + +If you want to try this out, you'll need to change the filename regex +patterns to match where you keep your Tor files. + +If you use emacs for editing Tor and nothing else, you could always just say: + +----- + (add-hook 'c-mode-hook + (lambda () + (font-lock-mode 1) + (set-variable 'show-trailing-whitespace t) + (set-variable 'indent-tabs-mode nil) + (set-variable 'c-basic-offset 2))) +----- + +There is probably a better way to do this. No, we are probably not going +to clutter the files with emacs stuff. + + +Doxygen +~~~~~~~ + +We use the 'doxygen' utility to generate documentation from our +source code. Here's how to use it: + + 1. Begin every file that should be documented with + /** + * \file filename.c + * \brief Short description of the file. + */ + + (Doxygen will recognize any comment beginning with /** as special.) + + 2. Before any function, structure, #define, or variable you want to + document, add a comment of the form: + + /** Describe the function's actions in imperative sentences. + * + * Use blank lines for paragraph breaks + * - and + * - hyphens + * - for + * - lists. + * + * Write <b>argument_names</b> in boldface. + * + * \code + * place_example_code(); + * between_code_and_endcode_commands(); + * \endcode + */ + + 3. Make sure to escape the characters "<", ">", "", "%" and "#" as "<", + ">", "\", "%", and "#". + + 4. To document structure members, you can use two forms: + + struct foo { + /** You can put the comment before an element; */ + int a; + int b; /**< Or use the less-than symbol to put the comment + * after the element. */ + }; + + 5. To generate documentation from the Tor source code, type: + + $ doxygen -g + + To generate a file called 'Doxyfile'. Edit that file and run + 'doxygen' to generate the API documentation. + + 6. See the Doxygen manual for more information; this summary just + scratches the surface. + diff --git a/doc/HACKING/HelpfulTools.txt b/doc/HACKING/HelpfulTools.txt deleted file mode 100644 index cf74063..0000000 --- a/doc/HACKING/HelpfulTools.txt +++ /dev/null @@ -1,303 +0,0 @@ -Useful tools ------------- - -These aren't strictly necessary for hacking on Tor, but they can help track -down bugs. - -Jenkins -~~~~~~~ - -https://jenkins.torproject.org - -Dmalloc -~~~~~~~ - -The dmalloc library will keep track of memory allocation, so you can find out -if we're leaking memory, doing any double-frees, or so on. - - dmalloc -l ~/dmalloc.log - (run the commands it tells you) - ./configure --with-dmalloc - -Valgrind -~~~~~~~~ - -valgrind --leak-check=yes --error-limit=no --show-reachable=yes src/or/tor - -(Note that if you get a zillion openssl warnings, you will also need to -pass --undef-value-errors=no to valgrind, or rebuild your openssl -with -DPURIFY.) - -Coverity -~~~~~~~~ - -Nick regularly runs the coverity static analyzer on the Tor codebase. - -The preprocessor define __COVERITY__ is used to work around instances -where coverity picks up behavior that we wish to permit. - -clang Static Analyzer -~~~~~~~~~~~~~~~~~~~~~ - -The clang static analyzer can be run on the Tor codebase using Xcode (WIP) -or a command-line build. - -The preprocessor define __clang_analyzer__ is used to work around instances -where clang picks up behavior that we wish to permit. - -clang Runtime Sanitizers -~~~~~~~~~~~~~~~~~~~~~~~~ - -To build the Tor codebase with the clang Address and Undefined Behavior -sanitizers, see the file contrib/clang/sanitize_blacklist.txt. - -Preprocessor workarounds for instances where clang picks up behavior that -we wish to permit are also documented in the blacklist file. - -Running lcov for unit test coverage -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Lcov is a utility that generates pretty HTML reports of test code coverage. -To generate such a report: - ------ - ./configure --enable-coverage - make - make coverage-html - $BROWSER ./coverage_html/index.html ------ - -This will run the tor unit test suite `./src/test/test` and generate the HTML -coverage code report under the directory ./coverage_html/. To change the -output directory, use `make coverage-html HTML_COVER_DIR=./funky_new_cov_dir`. - -Coverage diffs using lcov are not currently implemented, but are being -investigated (as of July 2014). - -Running the unit tests -~~~~~~~~~~~~~~~~~~~~~~ - -To quickly run all the tests distributed with Tor: ------ - make check ------ - -To run the fast unit tests only: ------ - make test ------ - -To selectively run just some tests (the following can be combined -arbitrarily): ------ - ./src/test/test <name_of_test> [<name of test 2>] ... - ./src/test/test <prefix_of_name_of_test>.. [<prefix_of_name_of_test2>..] ... - ./src/test/test :<name_of_excluded_test> [:<name_of_excluded_test2]... ------ - -To run all tests, including those based on Stem or Chutney: ------ - make test-full ------ - -To run all tests, including those based on Stem or Chutney that require a -working connection to the internet: ------ - make test-full-online ------ - -Running gcov for unit test coverage -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - ------ - ./configure --enable-coverage - make - make check - # or--- make test-full ? make test-full-online? - mkdir coverage-output - ./scripts/test/coverage coverage-output ------ - -(On OSX, you'll need to start with "--enable-coverage CC=clang".) - -Then, look at the .gcov files in coverage-output. '-' before a line means -that the compiler generated no code for that line. '######' means that the -line was never reached. Lines with numbers were called that number of times. - -If that doesn't work: - * Try configuring Tor with --disable-gcc-hardening - * You might need to run 'make clean' after you run './configure'. - -If you make changes to Tor and want to get another set of coverage results, -you can run "make reset-gcov" to clear the intermediary gcov output. - -If you have two different "coverage-output" directories, and you want to see -a meaningful diff between them, you can run: - ------ - ./scripts/test/cov-diff coverage-output1 coverage-output2 | less ------ - -In this diff, any lines that were visited at least once will have coverage -"1". This lets you inspect what you (probably) really want to know: which -untested lines were changed? Are there any new untested lines? - -Running integration tests -~~~~~~~~~~~~~~~~~~~~~~~~~ - -We have the beginnings of a set of scripts to run integration tests using -Chutney. To try them, set CHUTNEY_PATH to your chutney source directory, and -run "make test-network". - -We also have scripts to run integration tests using Stem. To try them, set -STEM_SOURCE_DIR to your Stem source directory, and run "test-stem". - -Profiling Tor with oprofile -~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -The oprofile tool runs (on Linux only!) to tell you what functions Tor is -spending its CPU time in, so we can identify performance bottlenecks. - -Here are some basic instructions - - - Build tor with debugging symbols (you probably already have, unless - you messed with CFLAGS during the build process). - - Build all the libraries you care about with debugging symbols - (probably you only care about libssl, maybe zlib and Libevent). - - Copy this tor to a new directory - - Copy all the libraries it uses to that dir too (ldd ./tor will - tell you) - - Set LD_LIBRARY_PATH to include that dir. ldd ./tor should now - show you it's using the libs in that dir - - Run that tor - - Reset oprofiles counters/start it - * "opcontrol --reset; opcontrol --start", if Nick remembers right. - - After a while, have it dump the stats on tor and all the libs - in that dir you created. - * "opcontrol --dump;" - * "opreport -l that_dir/*" - - Profit - -Generating and analyzing a callgraph -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -1. Run ./scripts/maint/generate_callgraph.sh . This will generate a - bunch of files in a new ./callgraph directory. - -2. Run ./scripts/maint/analyze_callgraph.py callgraph/src/*/* . This - will do a lot of graph operations and then dump out a new - "callgraph.pkl" file, containing data in Python's "pickle" format. - -3. Run ./scripts/maint/display_callgraph.py . It will display: - - the number of functions reachable from each function. - - all strongly-connnected components in the Tor callgraph - - the largest bottlenecks in the largest SCC in the Tor callgraph. - -Note that currently the callgraph generator can't detect calls that pass -through function pointers. - -Getting emacs to edit Tor source properly -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Nick likes to put the following snippet in his .emacs file: - ------ - (add-hook 'c-mode-hook - (lambda () - (font-lock-mode 1) - (set-variable 'show-trailing-whitespace t) - - (let ((fname (expand-file-name (buffer-file-name)))) - (cond - ((string-match "^/home/nickm/src/libevent" fname) - (set-variable 'indent-tabs-mode t) - (set-variable 'c-basic-offset 4) - (set-variable 'tab-width 4)) - ((string-match "^/home/nickm/src/tor" fname) - (set-variable 'indent-tabs-mode nil) - (set-variable 'c-basic-offset 2)) - ((string-match "^/home/nickm/src/openssl" fname) - (set-variable 'indent-tabs-mode t) - (set-variable 'c-basic-offset 8) - (set-variable 'tab-width 8)) - )))) ------ - -You'll note that it defaults to showing all trailing whitespace. The "cond" -test detects whether the file is one of a few C free software projects that I -often edit, and sets up the indentation level and tab preferences to match -what they want. - -If you want to try this out, you'll need to change the filename regex -patterns to match where you keep your Tor files. - -If you use emacs for editing Tor and nothing else, you could always just say: - ------ - (add-hook 'c-mode-hook - (lambda () - (font-lock-mode 1) - (set-variable 'show-trailing-whitespace t) - (set-variable 'indent-tabs-mode nil) - (set-variable 'c-basic-offset 2))) ------ - -There is probably a better way to do this. No, we are probably not going -to clutter the files with emacs stuff. - - -Doxygen -~~~~~~~ - -We use the 'doxygen' utility to generate documentation from our -source code. Here's how to use it: - - 1. Begin every file that should be documented with - /** - * \file filename.c - * \brief Short description of the file. - */ - - (Doxygen will recognize any comment beginning with /** as special.) - - 2. Before any function, structure, #define, or variable you want to - document, add a comment of the form: - - /** Describe the function's actions in imperative sentences. - * - * Use blank lines for paragraph breaks - * - and - * - hyphens - * - for - * - lists. - * - * Write <b>argument_names</b> in boldface. - * - * \code - * place_example_code(); - * between_code_and_endcode_commands(); - * \endcode - */ - - 3. Make sure to escape the characters "<", ">", "", "%" and "#" as "<", - ">", "\", "%", and "#". - - 4. To document structure members, you can use two forms: - - struct foo { - /** You can put the comment before an element; */ - int a; - int b; /**< Or use the less-than symbol to put the comment - * after the element. */ - }; - - 5. To generate documentation from the Tor source code, type: - - $ doxygen -g - - To generate a file called 'Doxyfile'. Edit that file and run - 'doxygen' to generate the API documentation. - - 6. See the Doxygen manual for more information; this summary just - scratches the surface. - diff --git a/doc/HACKING/HowToReview.md b/doc/HACKING/HowToReview.md new file mode 100644 index 0000000..d6b40db --- /dev/null +++ b/doc/HACKING/HowToReview.md @@ -0,0 +1,85 @@ +How to review a patch +===================== + +Some folks have said that they'd like to review patches more often, but they +don't know how. + +So, here are a bunch of things to check for when reviewing a patch! + +Note that if you can't do every one of these, that doesn't mean you can't do +a good review! Just make it clear what you checked for and what you didn't. + + +Top-level smell-checks +---------------------- + +(Difficulty: easy) + +Does it compile with --enable-gcc-warnings? + +Does 'make check-spaces' pass? + +Does it have a reasonable amount of tests? Do they pass? Do they leak +memory? + +Do all the new functions, global variables, types, and structure members have +documentation? + +Do all the functions, global variables, types, and structure members with +modified behavior have modified documentation? + +Do all the new torrc options have documentation? + +If this changes Tor's behavior on the wire, is there a design proposal? + + + +Let's look at the code! +----------------------- + +Does the code conform to CodingStandards.txt? + +Does the code leak memory? + +If two or more pointers ever point to the same object, is it clear which +pointer "owns" the object? + +Are all allocated resources freed? + +Are all pointers that should be const, const? + +Are #defines used for 'magic' numbers? + +Can you understand what the code is trying to do? + +Can you convince yourself that the code really does that? + +Is there duplicated code that could be turned into a function? + + +Let's look at the documentation! +-------------------------------- + +Does the documentation confirm to CodingStandards.txt? + +Does it make sense? + +Can you predict what the function will do from its documentation? + + +Let's think about security! +--------------------------- + +If there are any arrays, buffers, are you 100% sure that they cannot +overflow? + +If there is any integer math, can it overflow or underflow? + +If there are any allocations, are you sure there are corresponding +deallocations? + +Is there a safer pattern that could be used in any case? + +Have they used one of the Forbidden Functions? + +(Also see your favorite secure C programming guides.) diff --git a/doc/HACKING/HowToReview.txt b/doc/HACKING/HowToReview.txt deleted file mode 100644 index d6b40db..0000000 --- a/doc/HACKING/HowToReview.txt +++ /dev/null @@ -1,85 +0,0 @@ -How to review a patch -===================== - -Some folks have said that they'd like to review patches more often, but they -don't know how. - -So, here are a bunch of things to check for when reviewing a patch! - -Note that if you can't do every one of these, that doesn't mean you can't do -a good review! Just make it clear what you checked for and what you didn't. - - -Top-level smell-checks ----------------------- - -(Difficulty: easy) - -Does it compile with --enable-gcc-warnings? - -Does 'make check-spaces' pass? - -Does it have a reasonable amount of tests? Do they pass? Do they leak -memory? - -Do all the new functions, global variables, types, and structure members have -documentation? - -Do all the functions, global variables, types, and structure members with -modified behavior have modified documentation? - -Do all the new torrc options have documentation? - -If this changes Tor's behavior on the wire, is there a design proposal? - - - -Let's look at the code! ------------------------ - -Does the code conform to CodingStandards.txt? - -Does the code leak memory? - -If two or more pointers ever point to the same object, is it clear which -pointer "owns" the object? - -Are all allocated resources freed? - -Are all pointers that should be const, const? - -Are #defines used for 'magic' numbers? - -Can you understand what the code is trying to do? - -Can you convince yourself that the code really does that? - -Is there duplicated code that could be turned into a function? - - -Let's look at the documentation! --------------------------------- - -Does the documentation confirm to CodingStandards.txt? - -Does it make sense? - -Can you predict what the function will do from its documentation? - - -Let's think about security! ---------------------------- - -If there are any arrays, buffers, are you 100% sure that they cannot -overflow? - -If there is any integer math, can it overflow or underflow? - -If there are any allocations, are you sure there are corresponding -deallocations? - -Is there a safer pattern that could be used in any case? - -Have they used one of the Forbidden Functions? - -(Also see your favorite secure C programming guides.) diff --git a/doc/HACKING/README.1st b/doc/HACKING/README.1st deleted file mode 100644 index 3bee9ad..0000000 --- a/doc/HACKING/README.1st +++ /dev/null @@ -1,60 +0,0 @@ - -In this directory ------------------ - -This directory has helpful information about what you need to know to -hack on Tor! - -First, read 'GettingStarted.txt' to learn how to get a start in Tor -development. - -If you've decided to write a patch, 'CodingStandards.txt' will give -you a bunch of information about how we structure our code. - -It's important to get code right! Reading 'WritingTests.txt' will -tell you how to write and run tests in the Tor codebase. - -There are a bunch of other programs we use to help maintain and -develop the codebase: 'HelpfulTools.txt' can tell you how to use them -with Tor. - -If it's your job to put out Tor releases, see 'ReleasingTor.txt' so -that you don't miss any steps! - - - ------------------------ - -For full information on how Tor is supposed to work, look at the files in -https://gitweb.torproject.org/torspec.git/tree - -For an explanation of how to change Tor's design to work differently, look at -https://gitweb.torproject.org/torspec.git/blob_plain/HEAD:/proposals/001-pro... - -For the latest version of the code, get a copy of git, and - - git clone https://git.torproject.org/git/tor - -We talk about Tor on the tor-talk mailing list. Design proposals and -discussion belong on the tor-dev mailing list. We hang around on -irc.oftc.net, with general discussion happening on #tor and development -happening on #tor-dev. - -The other files in this "HACKING" directory may also be useful as you -get started working with Tor. - -Happy hacking! - -XXXXX also describe - -doc/HACKING/WritingTests.txt - -torguts.git - -torspec.git - -The design paper - -freehaven.net/anonbib - -XXXX describe these and add links. diff --git a/doc/HACKING/README.1st.md b/doc/HACKING/README.1st.md new file mode 100644 index 0000000..3bee9ad --- /dev/null +++ b/doc/HACKING/README.1st.md @@ -0,0 +1,60 @@ + +In this directory +----------------- + +This directory has helpful information about what you need to know to +hack on Tor! + +First, read 'GettingStarted.txt' to learn how to get a start in Tor +development. + +If you've decided to write a patch, 'CodingStandards.txt' will give +you a bunch of information about how we structure our code. + +It's important to get code right! Reading 'WritingTests.txt' will +tell you how to write and run tests in the Tor codebase. + +There are a bunch of other programs we use to help maintain and +develop the codebase: 'HelpfulTools.txt' can tell you how to use them +with Tor. + +If it's your job to put out Tor releases, see 'ReleasingTor.txt' so +that you don't miss any steps! + + + +----------------------- + +For full information on how Tor is supposed to work, look at the files in +https://gitweb.torproject.org/torspec.git/tree + +For an explanation of how to change Tor's design to work differently, look at +https://gitweb.torproject.org/torspec.git/blob_plain/HEAD:/proposals/001-pro... + +For the latest version of the code, get a copy of git, and + + git clone https://git.torproject.org/git/tor + +We talk about Tor on the tor-talk mailing list. Design proposals and +discussion belong on the tor-dev mailing list. We hang around on +irc.oftc.net, with general discussion happening on #tor and development +happening on #tor-dev. + +The other files in this "HACKING" directory may also be useful as you +get started working with Tor. + +Happy hacking! + +XXXXX also describe + +doc/HACKING/WritingTests.txt + +torguts.git + +torspec.git + +The design paper + +freehaven.net/anonbib + +XXXX describe these and add links. diff --git a/doc/HACKING/ReleasingTor.md b/doc/HACKING/ReleasingTor.md new file mode 100644 index 0000000..dcf551b --- /dev/null +++ b/doc/HACKING/ReleasingTor.md @@ -0,0 +1,125 @@ + +Putting out a new release +------------------------- + +Here are the steps Roger takes when putting out a new Tor release: + +1) Use it for a while, as a client, as a relay, as a hidden service, +and as a directory authority. See if it has any obvious bugs, and +resolve those. + +1.5) As applicable, merge the maint-X branch into the release-X branch. + +2) Gather the changes/* files into a changelog entry, rewriting many +of them and reordering to focus on what users and funders would find +interesting and understandable. + + 2.1) Make sure that everything that wants a bug number has one. + Make sure that everything which is a bugfix says what version + it was a bugfix on. + 2.2) Concatenate them. + 2.3) Sort them by section. Within each section, sort by "version it's + a bugfix on", else by numerical ticket order. + + 2.4) Clean them up: + + Standard idioms: + "Fixes bug 9999; bugfix on 0.3.3.3-alpha." + + One space after a period. + + Make stuff very terse + + Make sure each section name ends with a colon + + Describe the user-visible problem right away + + Mention relevant config options by name. If they're rare or unusual, + remind people what they're for + + Avoid starting lines with open-paren + + Present and imperative tense: not past. + + 'Relays', not 'servers' or 'nodes' or 'Tor relays'. + + "Stop FOOing", not "Fix a bug where we would FOO". + + Try not to let any given section be longer than about a page. Break up + long sections into subsections by some sort of common subtopic. This + guideline is especially important when organizing Release Notes for + new stable releases. + + If a given changes stanza showed up in a different release (e.g. + maint-0.2.1), be sure to make the stanzas identical (so people can + distinguish if these are the same change). + + 2.5) Merge them in. + + 2.6) Clean everything one last time. + + 2.7) Run ./scripts/maint/format_changelog.py to make it prettier. + +3) Compose a short release blurb to highlight the user-facing +changes. Insert said release blurb into the ChangeLog stanza. If it's +a stable release, add it to the ReleaseNotes file too. If we're adding +to a release-0.2.x branch, manually commit the changelogs to the later +git branches too. + +4) In maint-0.2.x, bump the version number in configure.ac and run + scripts/maint/updateVersions.pl to update version numbers in other + places, and commit. Then merge maint-0.2.x into release-0.2.x. + + (NOTE: To bump the version number, edit configure.ac, and then run + either make, or 'perl scripts/maint/updateVersions.pl', depending on + your version.) + +5) Make dist, put the tarball up somewhere, and tell #tor about it. Wait +a while to see if anybody has problems building it. Try to get Sebastian +or somebody to try building it on Windows. + +6) Get at least two of weasel/arma/Sebastian to put the new version number +in their approved versions list. + +7) Sign the tarball, then sign and push the git tag: + gpg -ba <the_tarball> + git tag -u <keyid> tor-0.2.x.y-status + git push origin tag tor-0.2.x.y-status + +8a) scp the tarball and its sig to the dist website, i.e. +/srv/dist-master.torproject.org/htdocs/ on dist-master. When you want +it to go live, you run "static-update-component dist.torproject.org" +on dist-master. + +8b) Edit "include/versions.wmi" and "Makefile" to note the new version. + +9) Email the packagers (cc'ing tor-assistants) that a new tarball is up. + The current list of packagers is: + {weasel,gk,mikeperry} at torproject dot org + {blueness} at gentoo dot org + {paul} at invizbox dot io + {ondrej.mikle} at gmail dot com + {lfleischer} at archlinux dot org + {tails-dev} at doum dot org + +10) Add the version number to Trac. To do this, go to Trac, log in, +select "Admin" near the top of the screen, then select "Versions" from +the menu on the left. At the right, there will be an "Add version" +box. By convention, we enter the version in the form "Tor: +0.2.2.23-alpha" (or whatever the version is), and we select the date as +the date in the ChangeLog. + +11) Forward-port the ChangeLog. + +12) Wait up to a day or two (for a development release), or until most +packages are up (for a stable release), and mail the release blurb and +changelog to tor-talk or tor-announce. + + (We might be moving to faster announcements, but don't announce until + the website is at least updated.) + +13) If it's a stable release, bump the version number in the maint-x.y.z + branch to "newversion-dev", and do a "merge -s ours" merge to avoid + taking that change into master. Do a similar 'merge -s theirs' + merge to get the change (and only that change) into release. (Some + of the build scripts require that maint merge cleanly into release.) diff --git a/doc/HACKING/ReleasingTor.txt b/doc/HACKING/ReleasingTor.txt deleted file mode 100644 index dcf551b..0000000 --- a/doc/HACKING/ReleasingTor.txt +++ /dev/null @@ -1,125 +0,0 @@ - -Putting out a new release -------------------------- - -Here are the steps Roger takes when putting out a new Tor release: - -1) Use it for a while, as a client, as a relay, as a hidden service, -and as a directory authority. See if it has any obvious bugs, and -resolve those. - -1.5) As applicable, merge the maint-X branch into the release-X branch. - -2) Gather the changes/* files into a changelog entry, rewriting many -of them and reordering to focus on what users and funders would find -interesting and understandable. - - 2.1) Make sure that everything that wants a bug number has one. - Make sure that everything which is a bugfix says what version - it was a bugfix on. - 2.2) Concatenate them. - 2.3) Sort them by section. Within each section, sort by "version it's - a bugfix on", else by numerical ticket order. - - 2.4) Clean them up: - - Standard idioms: - "Fixes bug 9999; bugfix on 0.3.3.3-alpha." - - One space after a period. - - Make stuff very terse - - Make sure each section name ends with a colon - - Describe the user-visible problem right away - - Mention relevant config options by name. If they're rare or unusual, - remind people what they're for - - Avoid starting lines with open-paren - - Present and imperative tense: not past. - - 'Relays', not 'servers' or 'nodes' or 'Tor relays'. - - "Stop FOOing", not "Fix a bug where we would FOO". - - Try not to let any given section be longer than about a page. Break up - long sections into subsections by some sort of common subtopic. This - guideline is especially important when organizing Release Notes for - new stable releases. - - If a given changes stanza showed up in a different release (e.g. - maint-0.2.1), be sure to make the stanzas identical (so people can - distinguish if these are the same change). - - 2.5) Merge them in. - - 2.6) Clean everything one last time. - - 2.7) Run ./scripts/maint/format_changelog.py to make it prettier. - -3) Compose a short release blurb to highlight the user-facing -changes. Insert said release blurb into the ChangeLog stanza. If it's -a stable release, add it to the ReleaseNotes file too. If we're adding -to a release-0.2.x branch, manually commit the changelogs to the later -git branches too. - -4) In maint-0.2.x, bump the version number in configure.ac and run - scripts/maint/updateVersions.pl to update version numbers in other - places, and commit. Then merge maint-0.2.x into release-0.2.x. - - (NOTE: To bump the version number, edit configure.ac, and then run - either make, or 'perl scripts/maint/updateVersions.pl', depending on - your version.) - -5) Make dist, put the tarball up somewhere, and tell #tor about it. Wait -a while to see if anybody has problems building it. Try to get Sebastian -or somebody to try building it on Windows. - -6) Get at least two of weasel/arma/Sebastian to put the new version number -in their approved versions list. - -7) Sign the tarball, then sign and push the git tag: - gpg -ba <the_tarball> - git tag -u <keyid> tor-0.2.x.y-status - git push origin tag tor-0.2.x.y-status - -8a) scp the tarball and its sig to the dist website, i.e. -/srv/dist-master.torproject.org/htdocs/ on dist-master. When you want -it to go live, you run "static-update-component dist.torproject.org" -on dist-master. - -8b) Edit "include/versions.wmi" and "Makefile" to note the new version. - -9) Email the packagers (cc'ing tor-assistants) that a new tarball is up. - The current list of packagers is: - {weasel,gk,mikeperry} at torproject dot org - {blueness} at gentoo dot org - {paul} at invizbox dot io - {ondrej.mikle} at gmail dot com - {lfleischer} at archlinux dot org - {tails-dev} at doum dot org - -10) Add the version number to Trac. To do this, go to Trac, log in, -select "Admin" near the top of the screen, then select "Versions" from -the menu on the left. At the right, there will be an "Add version" -box. By convention, we enter the version in the form "Tor: -0.2.2.23-alpha" (or whatever the version is), and we select the date as -the date in the ChangeLog. - -11) Forward-port the ChangeLog. - -12) Wait up to a day or two (for a development release), or until most -packages are up (for a stable release), and mail the release blurb and -changelog to tor-talk or tor-announce. - - (We might be moving to faster announcements, but don't announce until - the website is at least updated.) - -13) If it's a stable release, bump the version number in the maint-x.y.z - branch to "newversion-dev", and do a "merge -s ours" merge to avoid - taking that change into master. Do a similar 'merge -s theirs' - merge to get the change (and only that change) into release. (Some - of the build scripts require that maint merge cleanly into release.) diff --git a/doc/HACKING/WritingTests.md b/doc/HACKING/WritingTests.md new file mode 100644 index 0000000..2f59c9a --- /dev/null +++ b/doc/HACKING/WritingTests.md @@ -0,0 +1,403 @@ + +Writing tests for Tor: an incomplete guide +========================================== + +Tor uses a variety of testing frameworks and methodologies to try to +keep from introducing bugs. The major ones are: + + 1. Unit tests written in C and shipped with the Tor distribution. + + 2. Integration tests written in Python and shipped with the Tor + distribution. + + 3. Integration tests written in Python and shipped with the Stem + library. Some of these use the Tor controller protocol. + + 4. System tests written in Python and SH, and shipped with the + Chutney package. These work by running many instances of Tor + locally, and sending traffic through them. + + 5. The Shadow network simulator. + +How to run these tests +---------------------- + +=== The easy version + +To run all the tests that come bundled with Tor, run "make check" + +To run the Stem tests as well, fetch stem from the git repository, +set STEM_SOURCE_DIR to the checkout, and run "make test-stem". + +To run the Chutney tests as well, fetch chutney from the git repository, +set CHUTNEY_PATH to the checkout, and run "make test-network". + +To run all of the above, run "make test-full". + +To run all of the above, plus tests that require a working connection to the +internet, run "make test-full-online". + +=== Running particular subtests + +The Tor unit tests are divided into separate programs and a couple of +bundled unit test programs. + +Separate programs are easy. For example, to run the memwipe tests in +isolation, you just run ./src/test/test-memwipe . + +To run tests within the unit test programs, you can specify the name +of the test. The string ".." can be used as a wildcard at the end of the +test name. For example, to run all the cell format tests, enter +"./src/test/test cellfmt/..". To run + +Many tests that need to mess with global state run in forked subprocesses in +order to keep from contaminating one another. But when debugging a failing test, +you might want to run it without forking a subprocess. To do so, use the +"--no-fork" option with a single test. (If you specify it along with +multiple tests, they might interfere.) + +You can turn on logging in the unit tests by passing one of "--debug", +"--info", "--notice", or "--warn". By default only errors are displayed. + +Unit tests are divided into "./src/test/test" and "./src/test/test-slow". +The former are those that should finish in a few seconds; the latter tend to +take more time, and may include CPU-intensive operations, deliberate delays, +and stuff like that. + +=== Finding test coverage + +Test coverage is a measurement of which lines your tests actually visit. + +When you configure Tor with the --enable-coverage option, it should +build with support for coverage in the unit tests, and in a special +"tor-cov" binary. + +Then, run the tests you'd like to see coverage from. If you have old +coverage output, you may need to run "reset-gcov" first. + +Now you've got a bunch of files scattered around your build directories +called "*.gcda". In order to extract the coverage output from them, make a +temporary directory for them and run "./scripts/test/coverage ${TMPDIR}", +where ${TMPDIR} is the temporary directory you made. This will create a +".gcov" file for each source file under tests, containing that file's source +annotated with the number of times the tests hit each line. (You'll need to +have gcov installed.) + +You can get a summary of the test coverage for each file by running +"./scripts/test/cov-display ${TMPDIR}/*" . Each line lists the file's name, +the number of uncovered lines, the number of uncovered lines, and the +coverage percentage. + +For a summary of the test coverage for each _function_, run +"./scripts/test/cov-display -f ${TMPDIR}/*" . + +=== Comparing test coverage + +Sometimes it's useful to compare test coverage for a branch you're writing to +coverage from another branch (such as git master, for example). But you +can't run "diff" on the two coverage outputs directly, since the actual +number of times each line is executed aren't so important, and aren't wholly +deterministic. + +Instead, follow the instructions above for each branch, creating a separate +temporary directory for each. Then, run "./scripts/test/cov-diff ${D1} +${D2}", where D1 and D2 are the directories you want to compare. This will +produce a diff of the two directories, with all lines normalized to be either +covered or uncovered. + +To count new or modified uncovered lines in D2, you can run: + + "./scripts/test/cov-diff ${D1} ${D2}" | grep '^+ *#' |wc -l + + +What kinds of test should I write? +---------------------------------- + +Integration testing and unit testing are complementary: it's probably a +good idea to make sure that your code is hit by both if you can. + +If your code is very-low level, and its behavior is easily described in +terms of a relation between inputs and outputs, or a set of state +transitions, then it's a natural fit for unit tests. (If not, please +consider refactoring it until most of it _is_ a good fit for unit +tests!) + +If your code adds new externally visible functionality to Tor, it would +be great to have a test for that functionality. That's where +integration tests more usually come in. + +Unit and regression tests: Does this function do what it's supposed to? +----------------------------------------------------------------------- + +Most of Tor's unit tests are made using the "tinytest" testing framework. +You can see a guide to using it in the tinytest manual at + + https://github.com/nmathewson/tinytest/blob/master/tinytest-manual.md + +To add a new test of this kind, either edit an existing C file in src/test/, +or create a new C file there. Each test is a single function that must +be indexed in the table at the end of the file. We use the label "done:" as +a cleanup point for all test functions. + +(Make sure you read tinytest-manual.md before proceeding.) + +I use the term "unit test" and "regression tests" very sloppily here. + +=== A simple example + +Here's an example of a test function for a simple function in util.c: + + static void + test_util_writepid(void *arg) + { + (void) arg; + + char *contents = NULL; + const char *fname = get_fname("tmp_pid"); + unsigned long pid; + char c; + + write_pidfile(fname); + + contents = read_file_to_str(fname, 0, NULL); + tt_assert(contents); + + int n = sscanf(contents, "%lu\n%c", &pid, &c); + tt_int_op(n, OP_EQ, 1); + tt_int_op(pid, OP_EQ, getpid()); + + done: + tor_free(contents); + } + +This should look pretty familiar to you if you've read the tinytest +manual. One thing to note here is that we use the testing-specific +function "get_fname" to generate a file with respect to a temporary +directory that the tests use. You don't need to delete the file; +it will get removed when the tests are done. + +Also note our use of OP_EQ instead of == in the tt_int_op() calls. +We define OP_* macros to use instead of the binary comparison +operators so that analysis tools can more easily parse our code. +(Coccinelle really hates to see == used as a macro argument.) + +Finally, remember that by convention, all *_free() functions that +Tor defines are defined to accept NULL harmlessly. Thus, you don't +need to say "if (contents)" in the cleanup block. + +=== Exposing static functions for testing + +Sometimes you need to test a function, but you don't want to expose +it outside its usual module. + +To support this, Tor's build system compiles a testing version of +each module, with extra identifiers exposed. If you want to +declare a function as static but available for testing, use the +macro "STATIC" instead of "static". Then, make sure there's a +macro-protected declaration of the function in the module's header. + +For example, crypto_curve25519.h contains: + +#ifdef CRYPTO_CURVE25519_PRIVATE +STATIC int curve25519_impl(uint8_t *output, const uint8_t *secret, + const uint8_t *basepoint); +#endif + +The crypto_curve25519.c file and the test_crypto.c file both define +CRYPTO_CURVE25519_PRIVATE, so they can see this declaration. + +=== Mock functions for testing in isolation + +Often we want to test that a function works right, but the function to +be tested depends on other functions whose behavior is hard to observe, +or which require a working Tor network, or something like that. + +To write tests for this case, you can replace the underlying functions +with testing stubs while your unit test is running. You need to declare +the underlying function as 'mockable', as follows: + + MOCK_DECL(returntype, functionname, (argument list)); + +and then later implement it as: + + MOCK_IMPL(returntype, functionname, (argument list)) + { + /* implementation here */ + } + +For example, if you had a 'connect to remote server' function, you could +declare it as: + + + MOCK_DECL(int, connect_to_remote, (const char *name, status_t *status)); + +When you declare a function this way, it will be declared as normal in +regular builds, but when the module is built for testing, it is declared +as a function pointer initialized to the actual implementation. + +In your tests, if you want to override the function with a temporary +replacement, you say: + + MOCK(functionname, replacement_function_name); + +And later, you can restore the original function with: + + UNMOCK(functionname); + +For more information, see the definitions of this mocking logic in +testsupport.h. + +=== Okay but what should my tests actually do? + +We talk above about "test coverage" -- making sure that your tests visit +every line of code, or every branch of code. But visiting the code isn't +enough: we want to verify that it's correct. + +So when writing tests, try to make tests that should pass with any correct +implementation of the code, and that should fail if the code doesn't do what +it's supposed to do. + +You can write "black-box" tests or "glass-box" tests. A black-box test is +one that you write without looking at the structure of the function. A +glass-box one is one you implement while looking at how the function is +implemented. + +In either case, make sure to consider common cases *and* edge cases; success +cases and failure csaes. + +For example, consider testing this function: + + /** Remove all elements E from sl such that E==element. Preserve + * the order of any elements before E, but elements after E can be + * rearranged. + */ + void smartlist_remove(smartlist_t *sl, const void *element); + +In order to test it well, you should write tests for at least all of the +following cases. (These would be black-box tests, since we're only looking +at the declared behavior for the function: + + * Remove an element that is in the smartlist. + * Remove an element that is not in the smartlist. + * Remove an element that appears in the smartlist more than once. + +And your tests should verify that it behaves correct. At minimum, you should +test: + + * That other elements before E are in the same order after you call the + functions. + * That the target element is really removed. + * That _only_ the target element is removed. + +When you consider edge cases, you might try: + + * Remove an element from an empty list. + * Remove an element from a singleton list containing that element. + * Remove an element for a list containing several instances of that + element, and nothing else. + +Now let's look at the implementation: + + void + smartlist_remove(smartlist_t *sl, const void *element) + { + int i; + if (element == NULL) + return; + for (i=0; i < sl->num_used; i++) + if (sl->list[i] == element) { + sl->list[i] = sl->list[--sl->num_used]; /* swap with the end */ + i--; /* so we process the new i'th element */ + sl->list[sl->num_used] = NULL; + } + } + +Based on the implementation, we now see three more edge cases to test: + + * Removing NULL from the list. + * Removing an element from the end of the list + * Removing an element from a position other than the end of the list. + + +=== What should my tests NOT do? + +Tests shouldn't require a network connection. + +Whenever possible, tests shouldn't take more than a second. Put the test +into test/slow if it genuinely needs to be run. + +Tests should not alter global state unless they run with TT_FORK: Tests +should not require other tests to be run before or after them. + +Tests should not leak memory or other resources. To find out if your tests +are leaking memory, run them under valgrind (see HelpfulTools.txt for more +information on how to do that). + +When possible, tests should not be over-fit to the implementation. That is, +the test should verify that the documented behavior is implemented, but +should not break if other permissible behavior is later implemented. + + +=== Advanced techniques: Namespaces + +Sometimes, when you're doing a lot of mocking at once, it's convenient to +isolate your identifiers within a single namespace. If this were C++, we'd +already have namespaces, but for C, we do the best we can with macros and +token-pasting. + +We have some macros defined for this purpose in src/test/test.h. To use +them, you define NS_MODULE to a prefix to be used for your identifiers, and +then use other macros in place of identifier names. See src/test/test.h for +more documentation. + + +Integration tests: Calling Tor from the outside +----------------------------------------------- + +Some tests need to invoke Tor from the outside, and shouldn't run from the +same process as the Tor test program. Reasons for doing this might include: + + * Testing the actual behavior of Tor when run from the command line + * Testing that a crash-handler correctly logs a stack trace + * Verifying that violating a sandbox or capability requirement will + actually crash the program. + * Needing to run as root in order to test capability inheritance or + user switching. + +To add one of these, you generally want a new C program in src/test. Add it +to TESTS and noinst_PROGRAMS if it can run on its own and return success or +failure. If it needs to be invoked multiple times, or it needs to be +wrapped, add a new shell script to TESTS, and the new program to +noinst_PROGRAMS. If you need access to any environment variable from the +makefile (eg ${PYTHON} for a python interpreter), then make sure that the +makefile exports them. + +Writing integration tests with Stem +----------------------------------- + +The 'stem' library includes extensive unit tests for the Tor controller +protocol. + +For more information on writing new tests for stem, have a look around +the test/* directory in stem, and find a good example to emulate. You +might want to start with +https://gitweb.torproject.org/stem.git/tree/test/integ/control/controller.py +to improve Tor's test coverage. + +You can run stem tests from tor with "make test-stem", or see +https://stem.torproject.org/faq.html#how-do-i-run-the-tests . + +System testing with Chutney +--------------------------- + +The 'chutney' program configures and launches a set of Tor relays, +authorities, and clients on your local host. It has a 'test network' +functionality to send traffic through them and verify that the traffic +arrives correctly. + +You can write new test networks by adding them to 'networks'. To add +them to Tor's tests, add them to the test-network or test-network-all +targets in Makefile.am. + +(Adding new kinds of program to chutney will still require hacking the +code.) diff --git a/doc/HACKING/WritingTests.txt b/doc/HACKING/WritingTests.txt deleted file mode 100644 index 2f59c9a..0000000 --- a/doc/HACKING/WritingTests.txt +++ /dev/null @@ -1,403 +0,0 @@ - -Writing tests for Tor: an incomplete guide -========================================== - -Tor uses a variety of testing frameworks and methodologies to try to -keep from introducing bugs. The major ones are: - - 1. Unit tests written in C and shipped with the Tor distribution. - - 2. Integration tests written in Python and shipped with the Tor - distribution. - - 3. Integration tests written in Python and shipped with the Stem - library. Some of these use the Tor controller protocol. - - 4. System tests written in Python and SH, and shipped with the - Chutney package. These work by running many instances of Tor - locally, and sending traffic through them. - - 5. The Shadow network simulator. - -How to run these tests ----------------------- - -=== The easy version - -To run all the tests that come bundled with Tor, run "make check" - -To run the Stem tests as well, fetch stem from the git repository, -set STEM_SOURCE_DIR to the checkout, and run "make test-stem". - -To run the Chutney tests as well, fetch chutney from the git repository, -set CHUTNEY_PATH to the checkout, and run "make test-network". - -To run all of the above, run "make test-full". - -To run all of the above, plus tests that require a working connection to the -internet, run "make test-full-online". - -=== Running particular subtests - -The Tor unit tests are divided into separate programs and a couple of -bundled unit test programs. - -Separate programs are easy. For example, to run the memwipe tests in -isolation, you just run ./src/test/test-memwipe . - -To run tests within the unit test programs, you can specify the name -of the test. The string ".." can be used as a wildcard at the end of the -test name. For example, to run all the cell format tests, enter -"./src/test/test cellfmt/..". To run - -Many tests that need to mess with global state run in forked subprocesses in -order to keep from contaminating one another. But when debugging a failing test, -you might want to run it without forking a subprocess. To do so, use the -"--no-fork" option with a single test. (If you specify it along with -multiple tests, they might interfere.) - -You can turn on logging in the unit tests by passing one of "--debug", -"--info", "--notice", or "--warn". By default only errors are displayed. - -Unit tests are divided into "./src/test/test" and "./src/test/test-slow". -The former are those that should finish in a few seconds; the latter tend to -take more time, and may include CPU-intensive operations, deliberate delays, -and stuff like that. - -=== Finding test coverage - -Test coverage is a measurement of which lines your tests actually visit. - -When you configure Tor with the --enable-coverage option, it should -build with support for coverage in the unit tests, and in a special -"tor-cov" binary. - -Then, run the tests you'd like to see coverage from. If you have old -coverage output, you may need to run "reset-gcov" first. - -Now you've got a bunch of files scattered around your build directories -called "*.gcda". In order to extract the coverage output from them, make a -temporary directory for them and run "./scripts/test/coverage ${TMPDIR}", -where ${TMPDIR} is the temporary directory you made. This will create a -".gcov" file for each source file under tests, containing that file's source -annotated with the number of times the tests hit each line. (You'll need to -have gcov installed.) - -You can get a summary of the test coverage for each file by running -"./scripts/test/cov-display ${TMPDIR}/*" . Each line lists the file's name, -the number of uncovered lines, the number of uncovered lines, and the -coverage percentage. - -For a summary of the test coverage for each _function_, run -"./scripts/test/cov-display -f ${TMPDIR}/*" . - -=== Comparing test coverage - -Sometimes it's useful to compare test coverage for a branch you're writing to -coverage from another branch (such as git master, for example). But you -can't run "diff" on the two coverage outputs directly, since the actual -number of times each line is executed aren't so important, and aren't wholly -deterministic. - -Instead, follow the instructions above for each branch, creating a separate -temporary directory for each. Then, run "./scripts/test/cov-diff ${D1} -${D2}", where D1 and D2 are the directories you want to compare. This will -produce a diff of the two directories, with all lines normalized to be either -covered or uncovered. - -To count new or modified uncovered lines in D2, you can run: - - "./scripts/test/cov-diff ${D1} ${D2}" | grep '^+ *#' |wc -l - - -What kinds of test should I write? ----------------------------------- - -Integration testing and unit testing are complementary: it's probably a -good idea to make sure that your code is hit by both if you can. - -If your code is very-low level, and its behavior is easily described in -terms of a relation between inputs and outputs, or a set of state -transitions, then it's a natural fit for unit tests. (If not, please -consider refactoring it until most of it _is_ a good fit for unit -tests!) - -If your code adds new externally visible functionality to Tor, it would -be great to have a test for that functionality. That's where -integration tests more usually come in. - -Unit and regression tests: Does this function do what it's supposed to? ------------------------------------------------------------------------ - -Most of Tor's unit tests are made using the "tinytest" testing framework. -You can see a guide to using it in the tinytest manual at - - https://github.com/nmathewson/tinytest/blob/master/tinytest-manual.md - -To add a new test of this kind, either edit an existing C file in src/test/, -or create a new C file there. Each test is a single function that must -be indexed in the table at the end of the file. We use the label "done:" as -a cleanup point for all test functions. - -(Make sure you read tinytest-manual.md before proceeding.) - -I use the term "unit test" and "regression tests" very sloppily here. - -=== A simple example - -Here's an example of a test function for a simple function in util.c: - - static void - test_util_writepid(void *arg) - { - (void) arg; - - char *contents = NULL; - const char *fname = get_fname("tmp_pid"); - unsigned long pid; - char c; - - write_pidfile(fname); - - contents = read_file_to_str(fname, 0, NULL); - tt_assert(contents); - - int n = sscanf(contents, "%lu\n%c", &pid, &c); - tt_int_op(n, OP_EQ, 1); - tt_int_op(pid, OP_EQ, getpid()); - - done: - tor_free(contents); - } - -This should look pretty familiar to you if you've read the tinytest -manual. One thing to note here is that we use the testing-specific -function "get_fname" to generate a file with respect to a temporary -directory that the tests use. You don't need to delete the file; -it will get removed when the tests are done. - -Also note our use of OP_EQ instead of == in the tt_int_op() calls. -We define OP_* macros to use instead of the binary comparison -operators so that analysis tools can more easily parse our code. -(Coccinelle really hates to see == used as a macro argument.) - -Finally, remember that by convention, all *_free() functions that -Tor defines are defined to accept NULL harmlessly. Thus, you don't -need to say "if (contents)" in the cleanup block. - -=== Exposing static functions for testing - -Sometimes you need to test a function, but you don't want to expose -it outside its usual module. - -To support this, Tor's build system compiles a testing version of -each module, with extra identifiers exposed. If you want to -declare a function as static but available for testing, use the -macro "STATIC" instead of "static". Then, make sure there's a -macro-protected declaration of the function in the module's header. - -For example, crypto_curve25519.h contains: - -#ifdef CRYPTO_CURVE25519_PRIVATE -STATIC int curve25519_impl(uint8_t *output, const uint8_t *secret, - const uint8_t *basepoint); -#endif - -The crypto_curve25519.c file and the test_crypto.c file both define -CRYPTO_CURVE25519_PRIVATE, so they can see this declaration. - -=== Mock functions for testing in isolation - -Often we want to test that a function works right, but the function to -be tested depends on other functions whose behavior is hard to observe, -or which require a working Tor network, or something like that. - -To write tests for this case, you can replace the underlying functions -with testing stubs while your unit test is running. You need to declare -the underlying function as 'mockable', as follows: - - MOCK_DECL(returntype, functionname, (argument list)); - -and then later implement it as: - - MOCK_IMPL(returntype, functionname, (argument list)) - { - /* implementation here */ - } - -For example, if you had a 'connect to remote server' function, you could -declare it as: - - - MOCK_DECL(int, connect_to_remote, (const char *name, status_t *status)); - -When you declare a function this way, it will be declared as normal in -regular builds, but when the module is built for testing, it is declared -as a function pointer initialized to the actual implementation. - -In your tests, if you want to override the function with a temporary -replacement, you say: - - MOCK(functionname, replacement_function_name); - -And later, you can restore the original function with: - - UNMOCK(functionname); - -For more information, see the definitions of this mocking logic in -testsupport.h. - -=== Okay but what should my tests actually do? - -We talk above about "test coverage" -- making sure that your tests visit -every line of code, or every branch of code. But visiting the code isn't -enough: we want to verify that it's correct. - -So when writing tests, try to make tests that should pass with any correct -implementation of the code, and that should fail if the code doesn't do what -it's supposed to do. - -You can write "black-box" tests or "glass-box" tests. A black-box test is -one that you write without looking at the structure of the function. A -glass-box one is one you implement while looking at how the function is -implemented. - -In either case, make sure to consider common cases *and* edge cases; success -cases and failure csaes. - -For example, consider testing this function: - - /** Remove all elements E from sl such that E==element. Preserve - * the order of any elements before E, but elements after E can be - * rearranged. - */ - void smartlist_remove(smartlist_t *sl, const void *element); - -In order to test it well, you should write tests for at least all of the -following cases. (These would be black-box tests, since we're only looking -at the declared behavior for the function: - - * Remove an element that is in the smartlist. - * Remove an element that is not in the smartlist. - * Remove an element that appears in the smartlist more than once. - -And your tests should verify that it behaves correct. At minimum, you should -test: - - * That other elements before E are in the same order after you call the - functions. - * That the target element is really removed. - * That _only_ the target element is removed. - -When you consider edge cases, you might try: - - * Remove an element from an empty list. - * Remove an element from a singleton list containing that element. - * Remove an element for a list containing several instances of that - element, and nothing else. - -Now let's look at the implementation: - - void - smartlist_remove(smartlist_t *sl, const void *element) - { - int i; - if (element == NULL) - return; - for (i=0; i < sl->num_used; i++) - if (sl->list[i] == element) { - sl->list[i] = sl->list[--sl->num_used]; /* swap with the end */ - i--; /* so we process the new i'th element */ - sl->list[sl->num_used] = NULL; - } - } - -Based on the implementation, we now see three more edge cases to test: - - * Removing NULL from the list. - * Removing an element from the end of the list - * Removing an element from a position other than the end of the list. - - -=== What should my tests NOT do? - -Tests shouldn't require a network connection. - -Whenever possible, tests shouldn't take more than a second. Put the test -into test/slow if it genuinely needs to be run. - -Tests should not alter global state unless they run with TT_FORK: Tests -should not require other tests to be run before or after them. - -Tests should not leak memory or other resources. To find out if your tests -are leaking memory, run them under valgrind (see HelpfulTools.txt for more -information on how to do that). - -When possible, tests should not be over-fit to the implementation. That is, -the test should verify that the documented behavior is implemented, but -should not break if other permissible behavior is later implemented. - - -=== Advanced techniques: Namespaces - -Sometimes, when you're doing a lot of mocking at once, it's convenient to -isolate your identifiers within a single namespace. If this were C++, we'd -already have namespaces, but for C, we do the best we can with macros and -token-pasting. - -We have some macros defined for this purpose in src/test/test.h. To use -them, you define NS_MODULE to a prefix to be used for your identifiers, and -then use other macros in place of identifier names. See src/test/test.h for -more documentation. - - -Integration tests: Calling Tor from the outside ------------------------------------------------ - -Some tests need to invoke Tor from the outside, and shouldn't run from the -same process as the Tor test program. Reasons for doing this might include: - - * Testing the actual behavior of Tor when run from the command line - * Testing that a crash-handler correctly logs a stack trace - * Verifying that violating a sandbox or capability requirement will - actually crash the program. - * Needing to run as root in order to test capability inheritance or - user switching. - -To add one of these, you generally want a new C program in src/test. Add it -to TESTS and noinst_PROGRAMS if it can run on its own and return success or -failure. If it needs to be invoked multiple times, or it needs to be -wrapped, add a new shell script to TESTS, and the new program to -noinst_PROGRAMS. If you need access to any environment variable from the -makefile (eg ${PYTHON} for a python interpreter), then make sure that the -makefile exports them. - -Writing integration tests with Stem ------------------------------------ - -The 'stem' library includes extensive unit tests for the Tor controller -protocol. - -For more information on writing new tests for stem, have a look around -the test/* directory in stem, and find a good example to emulate. You -might want to start with -https://gitweb.torproject.org/stem.git/tree/test/integ/control/controller.py -to improve Tor's test coverage. - -You can run stem tests from tor with "make test-stem", or see -https://stem.torproject.org/faq.html#how-do-i-run-the-tests . - -System testing with Chutney ---------------------------- - -The 'chutney' program configures and launches a set of Tor relays, -authorities, and clients on your local host. It has a 'test network' -functionality to send traffic through them and verify that the traffic -arrives correctly. - -You can write new test networks by adding them to 'networks'. To add -them to Tor's tests, add them to the test-network or test-network-all -targets in Makefile.am. - -(Adding new kinds of program to chutney will still require hacking the -code.) diff --git a/doc/include.am b/doc/include.am index b235211..01a5ef2 100644 --- a/doc/include.am +++ b/doc/include.am @@ -39,13 +39,13 @@ EXTRA_DIST+= doc/asciidoc-helper.sh \ doc/state-contents.txt \ doc/torrc_format.txt \ doc/TUNING \ - doc/HACKING/README.1st \ - doc/HACKING/CodingStandards.txt \ - doc/HACKING/GettingStarted.txt \ - doc/HACKING/HelpfulTools.txt \ - doc/HACKING/HowToReview.txt \ - doc/HACKING/ReleasingTor.txt \ - doc/HACKING/WritingTests.txt + doc/HACKING/README.1st.md \ + doc/HACKING/CodingStandards.md \ + doc/HACKING/GettingStarted.md \ + doc/HACKING/HelpfulTools.md \ + doc/HACKING/HowToReview.md \ + doc/HACKING/ReleasingTor.md \ + doc/HACKING/WritingTests.md

docdir = @docdir@