Opened 7 years ago

Closed 7 years ago

#6520 closed enhancement (fixed)

read_from_old_location is set twice in router_reload_router_list_impl

Reported by: sysrqb Owned by:
Priority: Very Low Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: easy tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The second mutation appears to exist so that the conditional above it can exist. Is there a reason to check the return value?

read_from_old_location = 1;
log_notice(LD_DIR, "Couldn't read %s; trying to load routers from old "

"location %s.", fname, altname);

if ((store->mmap = tor_mmap_file(altname)))

read_from_old_location = 1;

Child Tickets

Attachments (1)

remove_cached_routers.patch (3.6 KB) - added by akshayhebbarys 7 years ago.
Removed all obsolete code related to cached-routers

Download all attachments as: .zip

Change History (11)

comment:1 in reply to:  description Changed 7 years ago by asn

Replying to sysrqb:

The second mutation appears to exist so that the conditional above it can exist. Is there a reason to check the return value?

read_from_old_location = 1;
log_notice(LD_DIR, "Couldn't read %s; trying to load routers from old "

"location %s.", fname, altname);

if ((store->mmap = tor_mmap_file(altname)))

read_from_old_location = 1;

Maybe the first

     read_from_old_location = 1;

shouldn't be there? That is, read_from_old_location should only be true if the alternative file was successfully mmapped.

comment:2 Changed 7 years ago by nickm

Hm. Keeping the second one makes sense.

That said, we moved from cached-routers to cached-descriptors back in 0.2.0.8-alpha. There is probably no point in supporting cached-routers at all any more; we could probably remove all of the logic related to it entirely in 0.2.4.x.

comment:3 in reply to:  2 Changed 7 years ago by sysrqb

Hmmm, alright, I guess I was reading it wrong - present tense instead of past. But in either case, given Nick's comment, should this ticket be adjusted to reflect removing all code related to cached-routers from the code base? Otherwise this ticket seems irrelevant and we can open a fresh ticket to track the clean up.

comment:4 Changed 7 years ago by nickm

Milestone: Tor: unspecified

comment:5 Changed 7 years ago by nickm

Keywords: easy added

comment:6 Changed 7 years ago by nickm

Keywords: tor-client added

comment:7 Changed 7 years ago by nickm

Component: Tor ClientTor

Changed 7 years ago by akshayhebbarys

Attachment: remove_cached_routers.patch added

Removed all obsolete code related to cached-routers

comment:8 Changed 7 years ago by akshayhebbarys

Status: newneeds_review

comment:9 Changed 7 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.2.5.x-final

comment:10 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged into master. Thanks!

Note: See TracTickets for help on using tickets.