Opened 9 years ago

Closed 7 years ago

#5605 closed defect (fixed)

Why is control_ports_write_to_file() called from options_act_reversible()?

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: easy tor-client
Cc: atagar, mikeperry Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

control_ports_write_to_file() writes out to a file every single time we setconf something. That's a minor issue, but ok.

What's weirder is that we call it from options_act_reversible(). There's a codepath (where the logs fail to initialize) where we end up refusing the new options but the control ports file remains written with the new options.

I think the better answer is to call it from options_act(). Also, it'd be slightly cleaner to pass 'options' in to the function.

Optimistically marking as easy (what could go wrong ;).

Child Tickets

Change History (8)

comment:1 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: unspecified

comment:2 Changed 8 years ago by nickm

Keywords: tor-client added

comment:3 Changed 8 years ago by nickm

Component: Tor ClientTor

comment:4 Changed 7 years ago by Ry

https://github.com/Ryman/tor/tree/bug5605

I didn't update the signatures for options_act* to take the new options as parameter as there are a lot of calls that aren't passing options down and are relying on get_options() to make their decisions. There's no real benefit to the codebase I think as options_act isn't really called anywhere else currently.

comment:5 Changed 7 years ago by nickm

Cc: atagar mikeperry added
Milestone: Tor: unspecifiedTor: 0.2.5.x-final
Status: newneeds_review

This looks correct to me. Damian, Mike, can you let me know if you can think of any problems this would cause?

comment:6 Changed 7 years ago by arma

Looks fine to me. I can't imagine it would cause any problems.

comment:7 Changed 7 years ago by nickm

Merged; thanks!

comment:8 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Looks like I merged but didn't close. Closing.

Note: See TracTickets for help on using tickets.