[tor-bugs] #24031 [Core Tor/Tor]: Protover.rs could use a better algorithm

Tor Bug Tracker & Wiki blackhole at torproject.org
Mon Mar 26 15:39:51 UTC 2018

#24031: Protover.rs could use a better algorithm
 Reporter:  nickm                                |          Owner:  isis
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Very High                            |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
 Severity:  Normal                               |     Resolution:
 Keywords:  rust, 033-must, protover, security,  |  Actual Points:  5
  033-triage-20180326, 033-included-20180326     |
Parent ID:                                       |         Points:  1
 Reviewer:  nickm                                |        Sponsor:
                                                 |  SponsorM-can

Comment (by chelseakomlo):

 Hi Isis,

 This looks great! Thanks for doing this and making these improvements, it
 is much cleaner!

 There are a few things I noticed when looking at this, they are mostly
 nits. I'll do a deeper look by the end of this week, but this is looking
 significantly better.

 1. If possible, it would be ideal to keep all FFI/C related code in
 ffi.rs. This way we can keep Rust/C code cleanly separated and if Rust
 needs to use these functions in the future, we don't need to do any
 refactoring or translation (for example, compute_for_old_tor and
 get_supported_protocols). It looks like in at least once place we get a
 string as a cstring in Rust (in supported for ProtoEntry) and then
 translate into a string- we should just keep this all as strings in Rust
 and then only translate in ffi.rs when we actually want to send something
 to C.
 2. I really like how you broke out errors into their own file. Is this
 something we should add to our guide to writing Rust in Tor?
 3. Is generating ToString implementations at runtime a common Rust
 practice? If not, document impl_to_string_for_proto_entry
 4. `supported` for ProtoEntry maybe should return an actual error instead
 of an empty string

Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24031#comment:15>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

More information about the tor-bugs mailing list