Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ticket33451 #1764

Closed
wants to merge 7 commits into from
Closed

Ticket33451 #1764

wants to merge 7 commits into from

Conversation

Reviewers

@sporksmith sporksmith

@teor2345 teor2345

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@nmathewson @coveralls @teor2345 @sporksmith
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Feb 25, 2020

No description provided.

nmathewson added 5 commits Feb 18, 2020
@nmathewson
Remember dirctory bw usage, and log it in the heartbeat
93af83e 
Closes ticket 32720.
@nmathewson
Make direclient_dump_total_dls() respect SafeLogging and pass tests.
ce9eeef 
Don't dump totals with anonymous purposes.  Additionally, don't dump
headers that have nothing underneath them.
@nmathewson
Use PRIu64 and TOR_PRIuSZ, to fix 32-bit build error.
84f2e28
@nmathewson
Merge remote-tracking branch 'tor-github/pr/1750/head'
11adf0f
@nmathewson
Add a script to install the scripts in scripts/git appropriately.
636e0fb 
To run this, say something like

   TOR_DEVTOOL_DIR=~/bin scripts/git/git-install-tools.sh all

To see what it would do, give it the -n flag.

To get help, give it the -h flag.
@coveralls
Copy link

@coveralls coveralls commented Feb 25, 2020 •
edited

Pull Request Test Coverage Report for Build 8278

  • 9 of 21 (42.86%) changed or added relevant lines in 3 files are covered.
  • 1300 unchanged lines in 21 files lost coverage.
  • Overall coverage increased (+0.03%) to 63.859%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/control/control_bootstrap.c 0 2 0.0%
src/feature/dirclient/dirclient.c 8 18 44.44%
Files with Coverage Reduction New Missed Lines %
src/core/or/or.h 1 88.89%
src/lib/confmgt/type_defs.c 2 98.6%
src/lib/net/address.h 2 73.33%
src/lib/confmgt/unitparse.c 3 95.16%
src/feature/dirparse/microdesc_parse.c 4 95.49%
src/lib/math/prob_distr.c 7 92.62%
src/feature/control/btrack_orconn_maps.c 8 85.71%
src/lib/err/torerr.c 10 82.93%
src/lib/log/util_bug.c 16 49.18%
src/feature/dirparse/authcert_parse.c 18 67.96%
Totals Coverage Status
Change from base Build 8202: 0.03%
Covered Lines: 50540
Relevant Lines: 79143
💛 - Coveralls

sporksmith
sporksmith reviewed Feb 25, 2020
View changes
scripts/git/git-install-tools.sh
@@ -0,0 +1,189 @@
#!/usr/bin/env bash

Copy link

@sporksmith sporksmith Feb 25, 2020

Consider using "strict mode": http://redsymbol.net/articles/unofficial-bash-strict-mode/. I see you use -e below, but consider also -u, -o, and pipefail. (I don't see any actual pipe usage here, but doesn't hurt to add it now for when it sneaks in later)

Copy link
Contributor

@teor2345 teor2345 Mar 12, 2020

If we're going to make this change, let's do it for all the git scripts, in a separate PR?

Copy link
Contributor

@teor2345 teor2345 Mar 12, 2020

I opened https://trac.torproject.org/projects/tor/ticket/33603 for this change in tor, I'll also open tickets for other projects.

Copy link

@sporksmith sporksmith Mar 13, 2020

SGTM

src/feature/dirclient/dirclient.c
* (For example, the number of bytes downloaded of purpose p while
* not fully bootstrapped is total_dl[p][false].)
**/
static uint64_t total_dl[DIR_PURPOSE_MAX_][2];
Copy link

@sporksmith sporksmith Feb 25, 2020

Consider using signed integers since these are intended as integers and not e.g. bit fields.

From https://google.github.io/styleguide/cppguide.html#Integer_Types:

Unsigned integers are good for representing bitfields and modular arithmetic. Because of historical accident, the C++ standard also uses unsigned integers to represent the size of containers - many members of the standards body believe this to be a mistake, but it is effectively impossible to fix at this point. The fact that unsigned arithmetic doesn't model the behavior of a simple integer, but is instead defined by the standard to model modular arithmetic (wrapping around on overflow/underflow), means that a significant class of bugs cannot be diagnosed by the compiler. In other cases, the defined behavior impedes optimization.

Copy link
Contributor Author

@nmathewson nmathewson Feb 25, 2020

This commit isn't actually part of this PR; the thing that updates github from Tor's git repository seems to be stalled.

Copy link

@sporksmith sporksmith Mar 13, 2020

Makes sense; disregard

nmathewson added 2 commits Feb 26, 2020
@nmathewson
fixup! Add a script to install the scripts in scripts/git appropriately.
0600f1d 
Make the alias invocation correct.
@nmathewson
fixup! Add a script to install the scripts in scripts/git appropriately.
47eafad 
Add "list-tor-branches" to the list of tools, now that #32121 is
merged into master.
@teor2345
Copy link
Contributor

@teor2345 teor2345 commented Mar 13, 2020

Squashed and merged to master.

@teor2345 teor2345 closed this Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment