#25914 closed defect (implemented)

dirserv: Remove dead code

Reported by: dgoulet Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-dirauth, deadcode, easy, 034-roadmap-subtask
Cc: Actual Points:
Parent ID: Points:
Reviewer: mikeperry Sponsor:

Description

While working on #25610, I've noticed dead code that uses dirserv_get_consensus().

In networkstatus.c, function networkstatus_set_current_consensus():

We parse the flavor and if it is unknown, we bail early with:

  if (flav < 0) {
    /* XXXX we don't handle unrecognized flavors yet. */
    log_warn(LD_BUG, "Unrecognized consensus flavor %s", flavor);
    return -2;
  }

But later in the function we have this if/else on the flavor name with a else {} statement that uses dirserv_get_consensus(). But we can't get into that else case since the first conditions are the only two flavors we can handle.

In between the first checks above and this if/else , the flavor can change as in we take the one from the given consensus but again, there is a check on if we can handle that flavor:

  if (flav != usable_consensus_flavor() &&
      !we_want_to_fetch_flavor(options, flav)) {

Bottom line, I think the else {} code is dead code. This simplifies the callgraph into the dirauth subsystem.

Child Tickets

Change History (6)

comment:1 Changed 18 months ago by dgoulet

Status: assignedneeds_review

See branch: ticket25914_034_01

comment:2 Changed 18 months ago by dgoulet

Reviewer: isis

comment:3 Changed 18 months ago by dgoulet

Reviewer: isismikeperry

comment:4 Changed 18 months ago by dgoulet

Parent ID: #25610

Turns out it has nothing to do with dirauth modularization. Unparenting.

comment:5 Changed 18 months ago by mikeperry

Status: needs_reviewmerge_ready

lgtm; is clearly dead code (I assume this means no changes file).

comment:6 Changed 18 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged! Thanks for the code and for the review!

Note: See TracTickets for help on using tickets.