Opened 6 months ago

Closed 4 months ago

#20921 closed task (implemented)

Refactor global_origin_circuit_list code into separate functions.

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-15
Cc: Actual Points: .1
Parent ID: #20822 Points: 1
Reviewer: dgoulet Sponsor:

Description


Child Tickets

TicketSummaryOwner
#21118circuit_get_global_origin_circuit_list() returns the wrong listnickm

Change History (11)

comment:1 Changed 5 months ago by nickm

  • Points set to 2

comment:2 Changed 5 months ago by nickm

  • Points changed from 2 to 1

comment:3 Changed 5 months ago by nickm

  • Actual Points set to .1
  • Status changed from new to needs_review

Implemented in my branch ticket20921, with a fix for #21118 too.

comment:4 Changed 4 months ago by dgoulet

  • Reviewer set to dgoulet
  • Status changed from needs_review to merge_ready

lgtm;

comment:5 Changed 4 months ago by dgoulet

  • Status changed from merge_ready to needs_revision

Oh wait backoff, my test weren't finish when I pressed submit. This makes make test explode.

cellqueue/circ_n_cells: [forking] Jan 10 10:11:39.450 [err] tor_assertion_failed_(): Bug: src/or/circuitlist.c:522: circuit_remove_from_origin_circuit_list: Assertion origin_idx <= smartlist_len(global_origin_circuit_list) failed; aborting. (on Tor 0.3.0.1-alpha-dev ef0559c3e318f16e)
Jan 10 10:11:39.450 [err] Bug: Assertion origin_idx <= smartlist_len(global_origin_circuit_list) failed in circuit_remove_from_origin_circuit_list at src/or/circuitlist.c:522. Stack trace: (on Tor 0.3.0.1-alpha-dev ef0559c3e318f16e)
circuituse/clean [...]
circuituse/onehop [...]
(and so on...)

comment:6 Changed 4 months ago by nickm

  • Keywords review-group-15 added

comment:7 Changed 4 months ago by nickm

  • Owner set to nickm
  • Status changed from needs_revision to accepted

setting owner

comment:8 Changed 4 months ago by nickm

  • Status changed from accepted to needs_revision

comment:9 Changed 4 months ago by nickm

I can't reproduce the above test failure; does it reproduce for you?

comment:10 Changed 4 months ago by dgoulet

  • Status changed from needs_revision to merge_ready

Yes #21118 missing commit definitely fixed it, I can't reproduce either.

comment:11 Changed 4 months ago by nickm

  • Resolution set to implemented
  • Status changed from merge_ready to closed
  • Type changed from defect to task

merged!

Note: See TracTickets for help on using tickets.