Opened 3 years ago

Closed 3 years ago

#20830 closed task (implemented)

Remove legacy guard algorithm code

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-guard
Cc: Actual Points:
Parent ID: #20822 Points: .5
Reviewer: Sponsor:

Description

Once we decide to merge the new guard code, there's no real point in keeping the old code around, since it's pretty thoroughly deprecated.

Child Tickets

Change History (13)

comment:1 Changed 3 years ago by nickm

As we do this, we should grep the code for every call to an instance of a function declared in entrynodes.h or bridges.h, and see if any of those functions can become static.

comment:2 Changed 3 years ago by dgoulet

Keywords: tor-guard added

comment:3 Changed 3 years ago by nickm

On my prop271_030_v1 branch, in commit 472a621ccc8a90bf90d8394210519974ad235848, I've wrapped all the legacy code in #ifdefs.

comment:4 Changed 3 years ago by asn

If we plan to remove the legacy code in two alpha releases of 0.3.0 anyway, I would not be opposed to removing it sooner (pre-merge), since I doubt someone is gonna rely on the legacy algorithm for those two alpha releases.

comment:5 Changed 3 years ago by nickm

If it's okay with you, I'd like to do it at-merge, in order to make the merge simpler.

comment:6 Changed 3 years ago by nickm

c52c47ae6f0da5 (merged to master) turns off compilation of this code. There's more to go, though: we'll have to actually remove it.

comment:7 Changed 3 years ago by nickm

Owner: set to nickm
Status: newassigned

comment:8 Changed 3 years ago by nickm

Points: .1

Planning to do this once 0.3.0.1-alpha has failed to crumble in people's hands for a couple of days.

comment:9 Changed 3 years ago by nickm

Points: .1.5

comment:10 Changed 3 years ago by nickm

Type: defecttask

comment:11 Changed 3 years ago by nickm

Status: assignedneeds_review

My branch remove_legacy_guards does this.

comment:12 Changed 3 years ago by asn

LGTM!

Please check my branch remove_legacy_guards for some more code deletions. I deleted three pieces of code which did:

     SMARTLIST_FOREACH_BEGIN(guard_contexts, guard_selection_t *, gs) {
-      if (!strcmp(gs->name, "legacy"))
-        continue;

Reading the code it should not be possible to instantiate a 'legacy' guard_selection_t anymore, so there is no point in checking for it, right?

Now the only part that mentions 'legacy' in entrynodes.c is:

    if (!strcmp(guard->selection_name, "legacy")) {
      ++n_errors;
      entry_guard_free(guard);
      continue;
    }

I think we might need to keep this there in case someone still has 'legacy' guards in their state file.

Check my branch and feel free to move this to merge_ready.

comment:13 Changed 3 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged your branch; thanks!

Note: See TracTickets for help on using tickets.