[tor-bugs] #25368 [Core Tor/Tor]: Update the Tor Rust coding standards for new types

[Tor Bug Tracker & Wiki]

#25368: Update the Tor Rust coding standards for new types
----------------------------------------+----------------------------------
 Reporter:  teor                        |          Owner:  teor
     Type:  defect                      |         Status:  merge_ready
 Priority:  Medium                      |      Milestone:  Tor:
                                        |  0.3.4.x-final
Component:  Core Tor/Tor                |        Version:
 Severity:  Normal                      |     Resolution:
 Keywords:  doc, rust, review-group-34  |  Actual Points:
Parent ID:  #23061                      |         Points:  0.5
 Reviewer:  isis                        |        Sponsor:
----------------------------------------+----------------------------------
Changes (by isis):

 * status:  needs_review => merge_ready


Comment:

 This looks great!  A couple thoughts:

 1) I only know enough about floating point arithmetic to stay away from
 it. That said, I was under the impression that
 [https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-
 numbers-2012-edition/ different compilers (and/or different compiler
 flags) can treat bitwise-identical floating points differently], and I
 worry in general that—if we allow `f{16,32,64}`s to cross the FFI
 boundary, that the C compiler on one side will treat them differently to
 how the Rust compiler does. I'm not sure how much this matters for the
 applications where we're using floating points, which seem mostly
 localised to the system using them (i.e. we're not sending floats over the
 network and expecting others to interpret them exactly as we do, right?).

 2) I can't explain `Stringlist`; it's always confused me. The way I'd do
 it is to have direct conversion between a `smartlist_t` and a `Vec<T>`,
 where `T` is probably an opaque pointer to whatever type in C, ''or'' `T`
 is only allowed to be a `String` which we've copied from a non-NULL
 `char*` (e.g. `impl From<Stringlist> for Vec<String>`, or something, and
 then keep `Stringlist` private since internally it's a bunch of C types
 that we don't want propagating into our more Rusty code)… but also I have
 not thought this through as much as I should yet.

 I'm okay with merging this, but also I think we should probably be
 figuring out a better way to deal with `smartlist_t`←→`Vec<T>`
 conversions.

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