code style improvements for src/rust/protover/ffi.rs

[frewsxcv]

https://trac.torproject.org/projects/tor/ticket/26492#comment:2

[tlyu]

Thanks for the patches! Overall, these changes seem like helpful code style improvements. Which style guide (if any) did you refer to for these? Is it https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/guide.md ?

I would say these are more code style improvements rather than refactoring, because the content of functions hasn't really been rearranged to improve modularity.

I also made a few inline review comments.

[tlyu]

Why not proto_entry.supports_protocol(&protocol, &version) as i32? Or maybe that should be as c_int?

[tlyu]

Alternatively, maybe putting the if-else in a single line as in https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/expressions.md#single-line-if-else
might be good.

[frewsxcv]

Considering rustfmt didn't automatically put it on one line, I assume it's not considered "small", as per this from your link:

...it contains a single else clause, and is small.

[tlyu]

I think it's in expression context, because it ends up being the implicitly returned value, so the single line if-else is allowed. It's not part of a larger statement or expression on the same line though, so maybe it's in a gray area, especially given the counterexample later in that section. (There's not enough context to tell if the multiline if-else is the last expression of a larger block.)

[tlyu]

This seems like a good change, and seems to be more aligned with how a lot of idiomatic Rust code takes string parameters.

[frewsxcv]

Thanks for the patches! Overall, these changes seem like helpful code style improvements. Which style guide (if any) did you refer to for these? Is it https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/guide.md ?

Yep. The style changes were all made automatically via the rustfmt tool.

[chelseakomlo]

This PR looks good to me! Thanks for changing allocate_and_copy_string to take a str, this is a nice improvement.

[tlyu]

@frewsxcv would you be willing to write a changes file? (see https://trac.torproject.org/projects/tor/ticket/26492#comment:9) We can also do this for you if you like because you've allowed maintainers to update your pull request branch. This PR might be in the gray area of things that need a changes file but it does (backwards-compatibly) change an interface.

[coveralls]

Coverage decreased (-0.4%) to 59.478% when pulling 7585523 on frewsxcv:frewsxcv-stringlist-refactor into b556894 on torproject:master.