Opened 8 years ago

Closed 6 years ago

#8546 closed enhancement (implemented)

Make a copy-able connection-config type to limit copy burden of isolation flags, etc

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client, refactoring, 026-triaged-1, nickm-patch, andrea-review
Cc: rl1987@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


Right now, an increasingly large number of fields and flags are duplicated between port_cfg_t, listener_connection_t, and (say) entry_connection_t. Every field we add here needs to be added to every one of those types, and needs to be explicitly copied from each to the next during construction time.

It would make this code much more maintainable if there were a type that we just copied from object to object here.

Child Tickets

Change History (9)

comment:1 Changed 7 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

comment:2 Changed 6 years ago by rl1987

Cc: rl1987@… added

comment:3 Changed 6 years ago by nickm

Keywords: 026-triaged-1 026-deferrable added

comment:4 Changed 6 years ago by nickm

Branch "bug8546" in my public repo has a fix for this, but needs a changes file. Please review?

comment:5 Changed 6 years ago by nickm

Keywords: nickm-patch added; easy 026-deferrable removed
Status: newneeds_review

comment:6 Changed 6 years ago by nickm

Keywords: andrea-review added

comment:7 Changed 6 years ago by andrea


  • All this looks good to me


  • Ditto, and I should learn coccinelle


  • This one looks fine to me too; I presume the intent is to copy this from the listener_connection_t to the new incoming connection?


  • Looks fine


  • This looks okay to me


  • Looks good


  • Looks good to me


  • Hmm, I think we're okay here, but it looks like in some cases (i.e. type == CONN_TYPE_AP_LISTENER) we're copying the whole structure instead of just a few fields in the old code. Are we certain it won't ever carry along values that will cause problems?


  • s/simplifification/simplification/, surely

comment:8 Changed 6 years ago by nickm

Thanks for the review! I'm 99% sure that the CONN_TYPE_AP_LISTENER case is safe. It actually copied nearly all fields before, but set just a few of them in the non-listener case.

Fixing and merging to master.

comment:9 Changed 6 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed
Type: defectenhancement
Note: See TracTickets for help on using tickets.