read_from_old_location is set twice in router_reload_router_list_impl

[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;

[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.

[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.

[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.

[akshayhebbarys]

Removed all obsolete code related to cached-routers

[nickm]

Merged into master. Thanks!