From c3c37433772542773dc9dfc73c27cf13bd0b9ad6 Mon Sep 17 00:00:00 2001 From: Ben Morrison Date: Sun, 12 Jan 2020 14:08:59 -0500 Subject: [PATCH] clippy lints and very minor cleanup --- rtcoin-server/src/conn.rs | 17 +++++------ rtcoin-server/src/db.rs | 14 ++++----- rtcoin-server/src/err.rs | 27 +++++++---------- rtcoin-server/src/json.rs | 10 +++---- rtcoin-server/src/main.rs | 36 +++++++++++----------- rtcoin-server/src/query.rs | 53 ++++++++++++++------------------- rtcoin-server/src/tests/db.rs | 4 +-- rtcoin-server/src/tests/err.rs | 2 +- rtcoin-server/src/tests/json.rs | 4 +-- rtcoin-server/src/tests/user.rs | 4 +++ rtcoin-server/src/user.rs | 53 ++++++++++++++------------------- 11 files changed, 103 insertions(+), 121 deletions(-) diff --git a/rtcoin-server/src/conn.rs b/rtcoin-server/src/conn.rs index 57dffb9..6802bce 100644 --- a/rtcoin-server/src/conn.rs +++ b/rtcoin-server/src/conn.rs @@ -44,9 +44,8 @@ pub fn init(mut conn: UnixStream, pipe: mpsc::Sender) { }); let json_in: Value = json::from_str(&json_in, Some(&mut conn)).unwrap(); - match json_in["kind"].to_string().as_ref() { - "quit" => break, - _ => {} + if let "quit" = json_in["kind"].to_string().as_ref() { + break; } route(&mut conn, &json_in, &pipe); @@ -61,7 +60,7 @@ fn route(conn: &mut UnixStream, json_in: &Value, pipe: &mpsc::Sender) let (tx, rx) = mpsc::channel::(); let comm = json::to_comm(&json_in, tx); - if let None = comm { + if comm.is_none() { return; } @@ -86,7 +85,7 @@ fn route(conn: &mut UnixStream, json_in: &Value, pipe: &mpsc::Sender) if resp.is_none() { log::info!("Closing client connection"); let out = err::Resp::new( - 01, + 1, "Worker Error", "No response from worker. Closing connection.", ) @@ -100,18 +99,18 @@ fn route(conn: &mut UnixStream, json_in: &Value, pipe: &mpsc::Sender) } fn recv(recv: Result, conn: &mut UnixStream) -> Option { - return match recv { + match recv { Ok(val) => Some(val), Err(err) => { let err = format!("{}", err); - let out = err::Resp::new(01, "Worker Error", &err); + let out = err::Resp::new(1, "Worker Error", &err); let out = out.to_bytes(); conn.write_all(&out).unwrap(); log::error!("Error in Ledger Worker Response: {}", err); None } - }; + } } // Response when the connection worker receives an @@ -119,7 +118,7 @@ fn recv(recv: Result, conn: &mut UnixStream) -> Opti // "Query" actions. fn invalid_request(conn: &mut UnixStream, kind: &str) { let details = format!("\"{}\" is not an allowed request type", kind); - let msg = err::Resp::new(03, "Invalid Request", &details); + let msg = err::Resp::new(3, "Invalid Request", &details); let msg = msg.to_bytes(); log::error!("Received invalid request from client: {}", details); diff --git a/rtcoin-server/src/db.rs b/rtcoin-server/src/db.rs index 5bf2b78..8e95710 100644 --- a/rtcoin-server/src/db.rs +++ b/rtcoin-server/src/db.rs @@ -5,7 +5,7 @@ use std::{path::Path, sync::mpsc}; -use log::info; +use log; use rusqlite::{Connection, OpenFlags, NO_PARAMS}; @@ -119,14 +119,14 @@ impl Comm { pub fn kind(&self) -> &Kind { match &self.kind { - Some(kind) => return &kind, - None => return &Kind::Empty, + Some(kind) => &kind, + None => &Kind::Empty, } } pub fn args(&self) -> Vec { match &self.args { - Some(args) => return args.clone(), - None => return Vec::::new(), + Some(args) => args.clone(), + None => Vec::::new(), } } } @@ -173,7 +173,7 @@ impl DB { // process the incoming Comms. pub fn worker_thread(&self) -> Comm { while let Ok(comm) = self.pipe.recv() { - info!("Ledger Worker :: Received {:?}", comm); + log::info!("Ledger Worker :: Received {:?}", comm); match comm.kind { Some(Kind::Register) => user::register(comm.clone(), &self.conn), Some(Kind::Whoami) => query::whoami(comm.clone(), &self.conn), @@ -187,7 +187,7 @@ impl DB { Some(Kind::Resolve) => {} Some(Kind::Second) => {} Some(Kind::Query) => {} - Some(Kind::Disconnect) => return comm.clone(), + Some(Kind::Disconnect) => return comm, _ => continue, } } diff --git a/rtcoin-server/src/err.rs b/rtcoin-server/src/err.rs index 1cc5ecd..1dd04d4 100644 --- a/rtcoin-server/src/err.rs +++ b/rtcoin-server/src/err.rs @@ -3,16 +3,12 @@ // See LICENSE file for detailed license information. // -use std::{ - fmt, -}; +use std::fmt; -use log::{ - error, -}; +use log; // Used for quickly serializing an error into bytes -// (or string) so that it may be sent across the socket. +// (or string) so that it may be sent across the socket. // Current error codes: // 01: Worker error // 02: Could not parse request as JSON @@ -51,9 +47,7 @@ impl Resp { } pub fn to_bytes(&self) -> Vec { - format!("{:#?}", self) - .as_bytes() - .to_owned() + format!("{:#?}", self).as_bytes().to_owned() } pub fn code(&self) -> u32 { self.code @@ -69,9 +63,10 @@ impl Resp { // I found myself writing this same construction // a few times repeatedly. pub fn log_then_panic(context: &str, err: T) - where T: fmt::Debug - { - let msg = format!("{}: {:?}", context, err); - error!("{}", msg); - panic!("{}", msg); - } \ No newline at end of file +where + T: fmt::Debug, +{ + let msg = format!("{}: {:?}", context, err); + log::error!("{}", msg); + panic!("{}", msg); +} diff --git a/rtcoin-server/src/json.rs b/rtcoin-server/src/json.rs index 99290cc..79dcf24 100644 --- a/rtcoin-server/src/json.rs +++ b/rtcoin-server/src/json.rs @@ -9,7 +9,7 @@ use crate::db; use crate::db::Kind; use crate::err; -use log::error; +use log; use serde_json::Value; // Deserializes a JSON Value struct into a db::Comm, @@ -51,13 +51,13 @@ pub fn to_comm(json: &Value, tx: mpsc::Sender) -> Option { // TODO: This is an unnecessary function. I need to get rid of it // and just call serde_json::from_str() directly pub fn from_str(json_in: &str, conn: Option<&mut UnixStream>) -> Option { - return match serde_json::from_str(&json_in) { + match serde_json::from_str(&json_in) { Ok(val) => Some(val), Err(err) => { let err = format!("{}", err); - let out = err::Resp::new(02, "JSON Error", &err); + let out = err::Resp::new(2, "JSON Error", &err); - error!("\nError {}:\n{}\n{}", out.code(), out.kind(), out.details(),); + log::error!("\nError {}:\n{}\n{}", out.code(), out.kind(), out.details(),); let out = out.to_bytes(); if let Some(conn) = conn { @@ -65,5 +65,5 @@ pub fn from_str(json_in: &str, conn: Option<&mut UnixStream>) -> Option Result<(), Box> { logging::init(); - info!("rtcoin-server is initializing.\n"); + log::info!("rtcoin-server is initializing.\n"); eprintln!("\nrtcoin-server 0.1-dev"); eprintln!("\nPlease enter the ledger password:"); @@ -47,43 +47,43 @@ fn main() -> Result<(), Box> { eprintln!(); // Create communication channel to the ledger database, then // spawn the ledger worker to listen for query requests. - info!("Starting ledger worker..."); + log::info!("Starting ledger worker..."); let (tx, rx) = mpsc::channel::(); thread::spawn(move || spawn_ledger_worker(db_key, rx)); // If the socket exists already, remove it. let sock = Path::new(conn::SOCK); if fs::metadata(sock).is_ok() { - warn!("Socket {} already exists.", conn::SOCK); + log::warn!("Socket {} already exists.", conn::SOCK); fs::remove_file(sock)?; } // Handle SIGINT / ^C let ctrlc_tx = tx.clone(); ctrlc::set_handler(move || { - warn!("^C / SIGINT Caught. Cleaning up ..."); + log::warn!("^C / SIGINT Caught. Cleaning up ..."); if fs::metadata(sock).is_ok() { - info!("Removing socket file"); + log::info!("Removing socket file"); fs::remove_file(sock).unwrap(); } - info!("SIGINT: Sending disconnect signal to ledger worker queue"); + log::info!("SIGINT: Sending disconnect signal to ledger worker queue"); let (reply_tx, sigint_rx) = mpsc::channel::(); let db_disconnect_signal = db::Comm::new(Some(db::Kind::Disconnect), None, Some(reply_tx)); match ctrlc_tx.send(db_disconnect_signal) { Ok(_) => {} - Err(err) => error!( + Err(err) => log::error!( "SIGINT: Failed to send disconnect signal to ledger worker: {}", err ), } // Block to allow database to close sigint_rx.recv().unwrap_or_else(|error| { - warn!("{:?}", error); + log::warn!("{:?}", error); process::exit(1); }); - info!("¡Hasta luego!"); + log::info!("¡Hasta luego!"); process::exit(0); }) .unwrap_or_else(|error| { @@ -93,7 +93,7 @@ fn main() -> Result<(), Box> { // Bind to the socket. Spawn a new connection // worker thread for each client connection. - info!("Binding to socket: {}", conn::SOCK); + log::info!("Binding to socket: {}", conn::SOCK); spawn_for_connections(&sock, tx); // Tidy up @@ -104,7 +104,7 @@ fn main() -> Result<(), Box> { fn spawn_ledger_worker(mut db_key: String, rx: mpsc::Receiver) { // This next call opens the actual database connection. // It also creates the tables if they don't yet exist. - info!("Connecting to database: {}", db::PATH); + log::info!("Connecting to database: {}", db::PATH); let ledger = DB::connect(db::PATH, db_key.clone(), rx); db_key.zeroize(); @@ -113,15 +113,15 @@ fn spawn_ledger_worker(mut db_key: String, rx: mpsc::Receiver) { let ledger_worker = thread::Builder::new(); let ledger_worker = ledger_worker.name("Ledger Worker".into()); - info!("Starting ledger worker process..."); + log::info!("Starting ledger worker process..."); let worker_thread = ledger_worker .spawn(move || { // once the worker_thread() method returns, // begin cleanup. so the whole process can exit. let disconnect_comm = ledger.worker_thread(); match ledger.conn.close() { - Err(err) => error!("Error closing database connection: {:?}", err), - Ok(_) => info!("Database connection successfully closed"), + Err(err) => log::error!("Error closing database connection: {:?}", err), + Ok(_) => log::info!("Database connection successfully closed"), } // Once we've closed the DB connection, let the @@ -140,7 +140,7 @@ fn spawn_ledger_worker(mut db_key: String, rx: mpsc::Receiver) { // Block execution until the thread we just // spawned returns. - info!("Startup finished!"); + log::info!("Startup finished!"); worker_thread.join().unwrap_or_else(|error| { err::log_then_panic("Ledger Worker", error); panic!() // otherwise rustc complains about return type @@ -159,14 +159,14 @@ fn spawn_for_connections(sock: &Path, tx: mpsc::Sender) { // resource hogs. let thread_num = num_cpus::get() * 4; let pool = ThreadPool::with_name("Client Connection".into(), thread_num); - info!("Using pool of {} threads", thread_num); + log::info!("Using pool of {} threads", thread_num); while let Ok((conn, addr)) = lstnr.accept() { // This is the channel that allows // clients to communicate with the // ledger worker process. let trx = tx.clone(); - info!("New client connection: {:?}", addr); + log::info!("New client connection: {:?}", addr); pool.execute(move || { conn::init(conn, trx); }); diff --git a/rtcoin-server/src/query.rs b/rtcoin-server/src/query.rs index efe326f..d8399a3 100644 --- a/rtcoin-server/src/query.rs +++ b/rtcoin-server/src/query.rs @@ -3,10 +3,7 @@ // See LICENSE file for detailed license information. // -use log::{ - error, - info, -}; +use log; use rusqlite; use rusqlite::NO_PARAMS; @@ -15,17 +12,17 @@ use crate::err; // Accepts the comm of kind Whoami and arg of // vec["user", (username)] -// Responds with the public key associated +// Responds with the public key associated // with the account. pub fn whoami(comm: db::Comm, conn: &rusqlite::Connection) { let args = comm.args(); let query = "SELECT pubkey FROM users WHERE name = :name"; - - // The ownership system causes the channel to + + // The ownership system causes the channel to // be moved, so I had to clone it. - let origin_channel = comm.origin.unwrap().clone(); + let origin_channel = comm.origin.unwrap(); // Twice. - let origin_channel_clone = origin_channel.clone(); + let origin_channel_clone = origin_channel; // Prevent out-of-bounds panics // from malformed requests. @@ -36,31 +33,28 @@ pub fn whoami(comm: db::Comm, conn: &rusqlite::Connection) { }; let query_for_logs = format!("{}, {}", query, user); - info!("New query: {}", query_for_logs); + log::info!("New query: {}", query_for_logs); let mut rowstmt = conn.prepare(&query).unwrap(); - // If the query fails, we can return an err::Resp // as long as it's been serialized into a string. - // Lets us continue on without adding complex + // Lets us continue on without adding complex // execution branches for handling errors. - let row = rowstmt.query_row_named( - &[(":name", &user)], - |row| { - Ok(row.get(0).unwrap()) - }) + let row = rowstmt + .query_row_named(&[(":name", &user)], |row| Ok(row.get(0).unwrap())) .unwrap_or_else(|err| { let err = format!("Query failed: {}", err); - error!("{} :: {}", err, query_for_logs); - err::Resp::new(04, "Query Error", &err).to_string() + log::error!("{} :: {}", err, query_for_logs); + err::Resp::new(4, "Query Error", &err).to_string() }); - let reply = match row.contains("Query Error") { - true => db::Reply::Error(row), - false => db::Reply::Data(row), - }; + let reply = if row.contains("Query Error") { + db::Reply::Error(row) + } else { + db::Reply::Data(row) + }; if let Err(err) = origin_channel_clone.send(reply) { - error!("Failed to send reply: {}", err); + log::error!("Failed to send reply: {}", err); } } @@ -81,10 +75,7 @@ pub fn to_ledger_entry(mut stmt: rusqlite::Statement) -> rusqlite::Result>() - ) -} \ No newline at end of file + Ok(rows + .map(|row| row.unwrap()) + .collect::>()) +} diff --git a/rtcoin-server/src/tests/db.rs b/rtcoin-server/src/tests/db.rs index 6a8c7e7..94499da 100644 --- a/rtcoin-server/src/tests/db.rs +++ b/rtcoin-server/src/tests/db.rs @@ -27,7 +27,7 @@ fn worker_thread_spawn_send_recv_query_rows() { let stmt = "SELECT * FROM ledger WHERE Source = 'Bob'"; let stmt = db.conn.prepare(stmt).unwrap(); - if let Err(_) = query::to_ledger_entry(stmt) { + if query::to_ledger_entry(stmt).is_err() { panic!("failure in query_to_ledger_rows()"); } // Above, comm takes ownership of the previous @@ -85,5 +85,5 @@ fn comm_kind() { #[bench] fn comm_kind_bench(bn: &mut test::Bencher) { - bn.iter(|| comm_kind()) + bn.iter(comm_kind) } diff --git a/rtcoin-server/src/tests/err.rs b/rtcoin-server/src/tests/err.rs index cc7d977..a6bd4a4 100644 --- a/rtcoin-server/src/tests/err.rs +++ b/rtcoin-server/src/tests/err.rs @@ -20,7 +20,7 @@ fn msg_resp() { #[bench] fn msg_resp_bench(b: &mut test::Bencher) { - b.iter(|| msg_resp()) + b.iter(msg_resp) } #[test] diff --git a/rtcoin-server/src/tests/json.rs b/rtcoin-server/src/tests/json.rs index e026577..18bce52 100644 --- a/rtcoin-server/src/tests/json.rs +++ b/rtcoin-server/src/tests/json.rs @@ -33,7 +33,7 @@ fn test_from_string() { #[bench] fn bench_from_string(b: &mut test::Bencher) { - b.iter(|| test_from_string()) + b.iter(test_from_string); } #[test] @@ -87,5 +87,5 @@ fn test_json_to_comm() { #[bench] fn bench_json_to_comm(b: &mut test::Bencher) { - b.iter(|| test_json_to_comm()) + b.iter(test_json_to_comm) } diff --git a/rtcoin-server/src/tests/user.rs b/rtcoin-server/src/tests/user.rs index 41af352..6b7d152 100644 --- a/rtcoin-server/src/tests/user.rs +++ b/rtcoin-server/src/tests/user.rs @@ -3,6 +3,10 @@ // See LICENSE file for detailed license information. // +// Since tildecoin isn't supposed to be used for anything serious, +// the rounding issues in floating point arithmetic are acceptable. +#![allow(clippy::float_cmp)] + extern crate test; use std::sync::mpsc; diff --git a/rtcoin-server/src/user.rs b/rtcoin-server/src/user.rs index 1bb6a85..9bf6bd0 100644 --- a/rtcoin-server/src/user.rs +++ b/rtcoin-server/src/user.rs @@ -94,7 +94,7 @@ pub fn register(comm: db::Comm, db: &rusqlite::Connection) { Some(t) => t, None => return, }; - let query = format!("INSERT INTO users (name, pass, pubkey, balance, created, last_login) VALUES (:name, :pass, :pubkey, :balance, :created, :last_login)"); + let query = "INSERT INTO users (name, pass, pubkey, balance, created, last_login) VALUES (:name, :pass, :pubkey, :balance, :created, :last_login)"; let args = match &comm.args { Some(val) => val, None => return, @@ -103,15 +103,11 @@ pub fn register(comm: db::Comm, db: &rusqlite::Connection) { let pass = args[1].clone(); let pubkey = args[2].clone(); - match check_pass(&pass) { - Err(err) => { - match tx.send(db::Reply::Error(format!("{:?}", err))) { - Ok(_) => {} - Err(err) => log::warn!("{:?}", err), - } - return; + if let Err(err) = check_pass(&pass) { + if let Err(err) = tx.send(db::Reply::Error(format!("{:?}", err))) { + log::warn!("{:?}", err); } - Ok(_) => {} + return; } let mut pass = match bcrypt::hash(&pass, 12) { @@ -130,7 +126,7 @@ pub fn register(comm: db::Comm, db: &rusqlite::Connection) { user.set_pass(&pass); - let mut stmt = match db.prepare(&query) { + let mut stmt = match db.prepare(query) { Ok(st) => st, Err(err) => { let err = format!("Internal Error: {:?}", err); @@ -195,22 +191,19 @@ pub fn rename(comm: db::Comm, db: &rusqlite::Connection) { let mut pass = args[2].clone(); args[2].zeroize(); - match auth(&old_user, &pass, &db) { - true => { - log::info!( - "User {} authenticated for: username change to {}", - old_user, - new_user - ); - } - false => { - log::error!("Auth failed for user {}", old_user); - return; - } + if auth(&old_user, &pass, &db) { + log::info!( + "User {} authenticated for: username change to {}", + old_user, + new_user + ); + } else { + log::error!("Auth failed for user {}", old_user); + return; } - let stmt = format!("UPDATE users SET name = :new_user WHERE name = :old_user"); - let mut stmt = match db.prepare(&stmt) { + let stmt = "UPDATE users SET name = :new_user WHERE name = :old_user"; + let mut stmt = match db.prepare(stmt) { Ok(s) => s, Err(err) => { log::error!("Failed to prepare update username statement: {:?}", err); @@ -240,10 +233,10 @@ pub fn rename(comm: db::Comm, db: &rusqlite::Connection) { } pub fn auth(user: &str, pass: &str, db: &rusqlite::Connection) -> bool { - let pass_verify_stmt = format!("SELECT pass FROM users WHERE name = :user"); + let pass_verify_stmt = "SELECT pass FROM users WHERE name = :user"; let stored_pass: String = - match db.query_row_named(&pass_verify_stmt, &[(":user", &user)], |row| { + match db.query_row_named(pass_verify_stmt, &[(":user", &user)], |row| { match row.get::(0) { Ok(s) => Ok(s), Err(err) => { @@ -259,16 +252,16 @@ pub fn auth(user: &str, pass: &str, db: &rusqlite::Connection) -> bool { let pass_bytes = pass.bytes().collect::>(); match bcrypt::verify(&pass_bytes, &stored_pass) { - Ok(boolean) => return boolean, + Ok(boolean) => boolean, Err(err) => { log::error!("Failed to verify password hash: {:?}", err); - return false; + false } } } -pub fn send(comm: db::Comm, db: &rusqlite::Connection) { - let args = if let Some(args) = comm.args { +pub fn send(comm: db::Comm, _db: &rusqlite::Connection) { + let _args = if let Some(args) = comm.args { args } else { vec![]