Opened 9 months ago

Closed 5 months ago

#24030 closed defect (implemented)

Wrap types in protover.rs

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: rust
Cc: chelseakomlo, coreyf+tor@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Our rust protover implementation throws around HashSet and HashMap with wild abandon. We should probably wrap those types in struct declarations, to make the intent more clear.

Child Tickets

Change History (9)

comment:1 Changed 7 months ago by frewsxcv

I haven't contributed to Tor before, but this looks like a ticket I could tackle! I'll take a stab a it…

comment:2 Changed 7 months ago by chelseakomlo

Hi, thanks for taking this on! This will be helpful. Let us know what questions you have or how we can help.

comment:3 Changed 7 months ago by frewsxcv

Cc: coreyf+tor@… added

comment:4 Changed 7 months ago by frewsxcv

Thanks for the offer for help! I'll have a branch for this ready soon.

I found some places where heap allocations happen unnecessarily and opened a separate ticket addressing it. Should I assign this ticket to someone?

Also, I'm not very familiar with the Trac workflow, but if it's too time consuming to submit/review/merge tickets with these small changesets, let me know if it's preferable to bundle them together with other commits.

comment:5 Changed 6 months ago by nickm

frewsxcv, any progress here?

comment:6 Changed 6 months ago by frewsxcv

Yep! I'll try to open a new issue w/ patch for this in the next day or two. Thanks for your patience! 🙏

comment:7 Changed 6 months ago by gk

#25067 it is.

comment:8 Changed 6 months ago by teor

Status: newneeds_review

comment:9 Changed 5 months ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged that; thanks, frewsxcv!

Note: See TracTickets for help on using tickets.