Opened 4 months ago

Closed 3 months ago

#22804 closed defect (implemented)

Refactor circuit_send_next_onionskin() to be less horribly large

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: refactor tor-hs
Cc: dgoulet Actual Points:
Parent ID: Points:
Reviewer: Sponsor: SponsorR

Description

There are three big sections to the abovementioned function: "CREATE", "EXTEND", and "Done." They should be separate functions.

See branch onionskin_refactor in my public repository. Said branch is optimized for readability :)

Child Tickets

Change History (4)

comment:1 Changed 4 months ago by nickm

Status: newneeds_review

comment:2 Changed 4 months ago by arma

Your branch looks good, aside from the easter egg that you fixed:

-    log_warn(LD_BUG, "Unhandled purpose %d with a chosen exit; "
+    log_warn(LD_BUG, "Unhandled purpose % dwith a chosen exit; "

I have taken your branch, rebased it to make it so your easter egg never happened, and added two more commits that I hope are helpful. It is now called onionskin_refactor still.

comment:3 Changed 4 months ago by dgoulet

Status: needs_reviewmerge_ready

lgtm; thanks!

(Of course, in an ideal world, we would unit tests the hell out of this if possible because circuit_send_next_onion_skin() has no unit test and it's pretty core.)

comment:4 Changed 3 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Added a changes file and merged to master.

Note: See TracTickets for help on using tickets.