Opened 10 months ago

Closed 8 months ago

#29196 closed defect (fixed)

circ: Remove p_mux and n_mux from circuit_t and or_circuit_t

Reported by: dgoulet Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-cmux, tor-circuit
Cc: Actual Points:
Parent ID: Points:
Reviewer: catalyst Sponsor:

Description

They are simply not used apart from assigning a pointer and asserting on the pointer depending on direction.

Complexity that is not needed.

Child Tickets

Change History (6)

comment:1 Changed 10 months ago by dgoulet

Status: assignedneeds_review

Branch: ticket29196_041_01

comment:2 Changed 9 months ago by dgoulet

Reviewer: catalyst

comment:3 Changed 9 months ago by catalyst

Status: needs_reviewneeds_information

This ticket says it targets 0.4.1 but the branch seems based on 0.4.0 (and has no pull request?). I'm not sure, which target branch is intended?

The code changes look good by visual inspection, and when I pushed the branch to my github, the only Travis failure was from the Cargo.lock issue that might already be fixed in master.

comment:4 in reply to:  3 ; Changed 9 months ago by dgoulet

Status: needs_informationneeds_review

Replying to catalyst:

This ticket says it targets 0.4.1 but the branch seems based on 0.4.0 (and has no pull request?). I'm not sure, which target branch is intended?

It is intended for master. No backport. At the time the branch was done, master was still 040.

comment:5 in reply to:  4 Changed 8 months ago by catalyst

Status: needs_reviewmerge_ready

Replying to dgoulet:

It is intended for master. No backport. At the time the branch was done, master was still 040.

Thanks! Looks good to me! I made a pull request at https://github.com/torproject/tor/pull/797 which passes CI. There are some coverage reductions, but looking at the build of the base revision, that might be coverage flapping due to nondeterminism.

comment:6 Changed 8 months ago by arma

Resolution: fixed
Status: merge_readyclosed

It is beautiful. I have merged it.

Note: See TracTickets for help on using tickets.