#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 21 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 20 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 20 months ago by frewsxcv

Cc: coreyf+tor@… added

comment:4 Changed 20 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 19 months ago by nickm

frewsxcv, any progress here?

comment:6 Changed 19 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 19 months ago by gk

#25067 it is.

comment:8 Changed 19 months ago by teor

Status: newneeds_review

comment:9 Changed 19 months ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Merged that; thanks, frewsxcv!

Note: See TracTickets for help on using tickets.