Opened 5 years ago

Closed 5 years ago

#15176 closed enhancement (fixed)

Refactor main loop for Shadow

Reported by: robgjansen Owned by:
Priority: Medium Milestone: Tor: 0.2.6.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: shadow-wants
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Let's make it easier to run Shadow. Shadow has its own main loop. When calling into Tor, Tor should not loop infinitely, else Shadow will 'freeze' in Tor's loop and not work properly. Separating the main loop into the main parts, and the then each iteration of the loop into a separate function will make this easier.

Patch attached, created from Tor master.

Child Tickets

Attachments (1)

0001-refactor-main-loop-to-improve-Shadow-integration.patch (4.7 KB) - added by robgjansen 5 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-final
Status: newneeds_revision

Hm. This code is C++, not C, an it doesn't match our docs convention. It's going to be pretty easy to reproduce though. Do you want this in master or 0.2.6?

(In C, we say void foo(void) not void foo().)

comment:2 Changed 5 years ago by robgjansen

If it's all the same to you, getting it into 0.2.6 would be great! (Sorry for the code style infractions.)

comment:3 Changed 5 years ago by nickm

Keywords: shadow-wants added
Milestone: Tor: 0.2.7.x-finalTor: 0.2.6.x-final
Status: needs_revisionneeds_review

See branch ticket15176 in my public repo. It's based on your branch, but I've tried to make it very simple to review, by putting most of the reformatting in its own commit.

Can somebody else also sign off on getting this into 0.2.6?

comment:4 Changed 5 years ago by yawning

Per IRC, both dgoulet and I think that this would be a good idea for 0.2.6.x. if it is trivial. (Supporting the research community is a Good Thing(TM)).

  • b78803f9f543c83bf50d0751eb160e80ed602eae "Extract main part of main loop into a separate function" Trivial refactor, ACK.
  • 92d04721a2a614e40d6393ccc64ff2e86b71d778 "remove a needless "if (1)" that was there for indentation; fix indentation." Formating change, removing a conditional that would be optimized out anyway, ACK.
  • ddb1889eb8bc128ec0fca35f15b1bf90d2b629ac "Add comments for new functions" Documentation change, ACK. (Remove the newline after the first "/" maybe).
  • a0f892f19094efd963564cf788be887f694be796 "Simplify the loop." Trivial change, ACK. (Since you're initializing loop_result at declaration time you could use a while loop instead of do/while, but that's a stylistic thing).

ACK, minor nitpicks that are a matter of personal taste. I'd say it's ok for 0.2.6.x.

comment:5 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Thanks; merged!

Note: See TracTickets for help on using tickets.