[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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

#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:
                                                 |  0.3.3.1-alpha
 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

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs