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

#21349: Refactor long functions in entrynodes.c (3) #202

Closed
wants to merge 8 commits into from

Conversation

Labels
None yet
Projects
None yet
4 participants
@rl1987
Copy link
Contributor

@rl1987 rl1987 commented Jul 3, 2018

https://trac.torproject.org/projects/tor/ticket/21349

@coveralls
Copy link

@coveralls coveralls commented Jul 3, 2018

Coverage Status

Coverage increased (+0.03%) to 59.529% when pulling f971b45 on rl1987:ticket21349_3_clean into a9628c0 on torproject:master.

* values are matching). Otherwise, return false.
*/
int
smartlist_shallow_eq(const smartlist_t *s1, const smartlist_t *s2)
Copy link
Contributor

@nmathewson nmathewson Jul 11, 2018

For consistency with smartlist_ints_eq and smartlist_strings_eq, this should probably be called smartlist_ptrs_eq

Copy link
Contributor Author

@rl1987 rl1987 Jul 18, 2018

Renamed in 8a18eba.

int
smartlist_shallow_eq(const smartlist_t *s1, const smartlist_t *s2)
{
if (s1 == NULL || s2 == NULL)
Copy link
Contributor

@nmathewson nmathewson Jul 11, 2018

if s1 == NULL && s2 == NULL I think this function should return 1.

Copy link
Contributor Author

@rl1987 rl1987 Jul 18, 2018

Fixed in ac40d0e.

@@ -404,6 +404,17 @@ get_remove_unlisted_guards_after_days(void)
DFLT_REMOVE_UNLISTED_GUARDS_AFTER_DAYS,
1, 365*10);
}

/**
* If a guard has been unlisted for this many seconds continuously, we
Copy link
Contributor

@nmathewson nmathewson Jul 11, 2018

Usually we'll document a function by using a verb to describe what it does, like "Return the number of seconds after which we remove a guard that..."

Copy link
Contributor Author

@rl1987 rl1987 Jul 18, 2018

Reworded in be9401d .

SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) {
/* XXXX #20827 check ed ID too */
const int is_listed = entry_guard_is_listed(gs, guard);

if (is_listed && ! guard->currently_listed) {
++n_changes;
Copy link
Contributor

@nmathewson nmathewson Jul 11, 2018

What's the point of these changes?

Copy link
Contributor Author

@rl1987 rl1987 Jul 18, 2018

None really, just a byproduct of some now-squashed fixup commits. Changed it back in 4a82c90 .

@tlyu
Copy link
Contributor

@tlyu tlyu commented Jul 25, 2018

Superseded by #246.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment