Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

network-devp2p Fix some clippy errors/warnings #9378

Merged
merged 3 commits into from
Aug 21, 2018
Merged

Conversation

niklasad1
Copy link
Collaborator

These errors/warnings are not fixed:

warning: the function has a cyclomatic complexity of 28
   --> util/network-devp2p/src/host.rs:678:2
    |
678 |       fn session_readable(&self, token: StreamToken, io: &IoContext<NetworkIoMessage>) {
    |  _____^
679 | |         let mut ready_data: Vec<ProtocolId> = Vec::new();
680 | |         let mut packet_data: Vec<(ProtocolId, PacketId, Vec<u8>)> = Vec::new();
681 | |         let mut kill = false;
...   |
823 | |         }
824 | |     }
    | |_____^
    |
    = note: #[warn(cyclomatic_complexity)] on by default
    = help: you could split it up into multiple smaller functions
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cyclomatic_complexity

warning: casting u8 to u32 may become silently lossy if types change
   --> util/network-devp2p/src/connection.rs:393:19
    |
393 |         let length = ((((hdec[0] as u32) << 8) + (hdec[1] as u32)) << 8) + (hdec[2] as u32);
    |                         ^^^^^^^^^^^^^^^^ help: try: `u32::from(hdec[0])`
    |
    = note: #[warn(cast_lossless)] on by default
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_lossless

warning: casting u8 to u32 may become silently lossy if types change
   --> util/network-devp2p/src/connection.rs:393:44
    |
393 |         let length = ((((hdec[0] as u32) << 8) + (hdec[1] as u32)) << 8) + (hdec[2] as u32);
    |                                                  ^^^^^^^^^^^^^^^^ help: try: `u32::from(hdec[1])`
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_lossless

warning: casting u8 to u32 may become silently lossy if types change
   --> util/network-devp2p/src/connection.rs:393:70
    |
393 |         let length = ((((hdec[0] as u32) << 8) + (hdec[1] as u32)) << 8) + (hdec[2] as u32);
    |                                                                            ^^^^^^^^^^^^^^^^ help: try: `u32::from(hdec[2])`
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_lossless

warning: large size difference between variants
  --> util/network-devp2p/src/session.rs:70:2
   |
70 |     Session(EncryptedConnection),
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: #[warn(large_enum_variant)] on by default
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#large_enum_variant
help: consider boxing the large fields to reduce the total size of the enum
   |
70 |     Session(Box<EncryptedConnection>),
   |             ^^^^^^^^^^^^^^^^^^^^^^^^

error: casting from `*const u8` to a more-strictly-aligned pointer (`*const u16`)
  --> util/network-devp2p/src/node_table.rs:78:25
   |
78 |                 let o: *const u16 = addr_bytes.as_ptr() as *const u16;
   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: #[deny(cast_ptr_alignment)] on by default
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_ptr_alignment

warning: you seem to be using .map() to clone the contents of an iterator, consider using `.cloned()`
   --> util/network-devp2p/src/node_table.rs:393:15
    |
393 |           let nodes = node_ids.into_iter()
    |  _____________________^
394 | |             .map(|id| self.nodes.get(&id).expect("self.nodes() only returns node IDs from self.nodes"))
395 | |             .take(MAX_NODES)
396 | |             .map(|node| node.clone())
    | |_____________________________________^
    |
    = note: #[warn(map_clone)] on by default
    = help: try
            node_ids.into_iter()
                        .map(|id| self.nodes.get(&id).expect("self.nodes() only returns node IDs from self.nodes"))
                        .take(MAX_NODES).cloned()
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#map_clone

error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type
   --> util/network-devp2p/src/node_table.rs:396:16
    |
396 |             .map(|node| node.clone())
    |                         ^^^^^^^^^^^^
    |
    = note: #[deny(clone_double_ref)] on by default
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#clone_double_ref
help: try dereferencing it
    |
396 |             .map(|node| &(*node).clone())
    |                         ^^^^^^^^^^^^^^^^
help: or try being explicit about what type to clone
    |
396 |             .map(|node| &node_table::Node::clone(node))
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: casting from `*mut libc::sockaddr` to a more-strictly-aligned pointer (`*const libc::sockaddr_in`)
   --> util/network-devp2p/src/ip_utils.rs:225:34
    |
225 |                 let sa: *const sockaddr_in = sa as *const sockaddr_in;
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_ptr_alignment

error: casting from `*mut libc::sockaddr` to a more-strictly-aligned pointer (`*const libc::sockaddr_in6`)
   --> util/network-devp2p/src/ip_utils.rs:233:35
    |
233 |                 let sa: *const sockaddr_in6 = sa as *const sockaddr_in6;
    |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#cast_ptr_alignment

error: aborting due to 4 previous errors

error: Could not compile `ethcore-network-devp2p`.

Remarks:

  • Node is not a clone type so I guess shallow copying makes sense

@niklasad1 niklasad1 added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M4-core ⛓ Core client code / Rust. labels Aug 20, 2018
@ordian
Copy link
Collaborator

ordian commented Aug 20, 2018

Node is not a clone type so I guess shallow copying makes sense

I guess we can safely remove .map(|node| node.clone()), since clone does not do what was probably intended, since node_table::json::Node used in Into::into constructs from a reference: impl<'a> From<&'a super::Node> for Node.

@@ -173,7 +173,7 @@ impl NetworkService {
}

/// Set the non-reserved peer mode.
pub fn set_non_reserved_mode(&self, mode: NonReservedPeerMode) {
pub fn set_non_reserved_mode(&self, mode: &NonReservedPeerMode) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make it Copy?

@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 21, 2018
@ordian ordian merged commit 8703449 into master Aug 21, 2018
@ordian ordian deleted the network-devp2p/clippy branch August 21, 2018 09:55
@5chdn 5chdn added this to the 2.1 milestone Aug 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants