From 2fd5676ff2eebdfc7e4417cef151596bd1c9e13b Mon Sep 17 00:00:00 2001 From: Jean Date: Mon, 13 Jul 2026 23:30:51 +0000 Subject: [PATCH] Clippy sweep: clean across the workspace - Box the oversized enum variants (Event::AccountRegistered, gossip Msg::Entry) so small events aren't sized to a full Account; serde is transparent so the wire/log format is unchanged. - Use the configured server description in the SERVER line (it was hardcoded and the config field went unread). - Derive the ChanServ SET label from the ChanSetting (drops toggle's 8th arg); rename inspircd from_us -> sourced. - allow the two framework-idiom lints with rationale (tonic's Status in authorize; the guest-rename handler's inherent arg count). - Auto-fixed map_or/contains/split_once style lints. --- chanserv/src/set.rs | 29 ++++++++++++++++++++++------- inspircd/src/lib.rs | 39 ++++++++++++++++++++------------------- nickserv/src/ghost.rs | 3 +++ nickserv/src/ungroup.rs | 2 +- src/engine/db.rs | 24 +++++++++++++----------- src/engine/mod.rs | 2 +- src/gossip.rs | 10 ++++++---- src/grpc.rs | 5 ++++- src/main.rs | 1 + 9 files changed, 71 insertions(+), 44 deletions(-) diff --git a/chanserv/src/set.rs b/chanserv/src/set.rs index 0fac711..987b335 100644 --- a/chanserv/src/set.rs +++ b/chanserv/src/set.rs @@ -42,18 +42,33 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: Err(_) => ctx.notice(me, from.uid, "Sorry, that didn't work. Please try again in a moment."), } } - Some("SIGNKICK") => toggle(me, from, ctx, db, chan, ChanSetting::SignKick, args.get(3).copied(), "SIGNKICK"), - Some("PRIVATE") => toggle(me, from, ctx, db, chan, ChanSetting::Private, args.get(3).copied(), "PRIVATE"), - Some("PEACE") => toggle(me, from, ctx, db, chan, ChanSetting::Peace, args.get(3).copied(), "PEACE"), - Some("SECUREOPS") => toggle(me, from, ctx, db, chan, ChanSetting::SecureOps, args.get(3).copied(), "SECUREOPS"), - Some("KEEPTOPIC") => toggle(me, from, ctx, db, chan, ChanSetting::KeepTopic, args.get(3).copied(), "KEEPTOPIC"), - Some("TOPICLOCK") => toggle(me, from, ctx, db, chan, ChanSetting::TopicLock, args.get(3).copied(), "TOPICLOCK"), + Some("SIGNKICK") => toggle(me, from, ctx, db, chan, ChanSetting::SignKick, args.get(3).copied()), + Some("PRIVATE") => toggle(me, from, ctx, db, chan, ChanSetting::Private, args.get(3).copied()), + Some("PEACE") => toggle(me, from, ctx, db, chan, ChanSetting::Peace, args.get(3).copied()), + Some("SECUREOPS") => toggle(me, from, ctx, db, chan, ChanSetting::SecureOps, args.get(3).copied()), + Some("KEEPTOPIC") => toggle(me, from, ctx, db, chan, ChanSetting::KeepTopic, args.get(3).copied()), + Some("TOPICLOCK") => toggle(me, from, ctx, db, chan, ChanSetting::TopicLock, args.get(3).copied()), _ => ctx.notice(me, from.uid, "Syntax: SET <#channel> FOUNDER | DESC | SIGNKICK {ON|OFF} | PRIVATE {ON|OFF} | PEACE {ON|OFF} | SECUREOPS {ON|OFF} | KEEPTOPIC {ON|OFF} | TOPICLOCK {ON|OFF}"), } } +// The SET keyword for a channel option, used in its messages. +fn label(setting: ChanSetting) -> &'static str { + match setting { + ChanSetting::SignKick => "SIGNKICK", + ChanSetting::Private => "PRIVATE", + ChanSetting::Peace => "PEACE", + ChanSetting::SecureOps => "SECUREOPS", + ChanSetting::KeepTopic => "KEEPTOPIC", + ChanSetting::TopicLock => "TOPICLOCK", + ChanSetting::BotGreet => "GREET", + ChanSetting::NoBot => "NOBOT", + } +} + // Flip one on/off channel option, reporting the new state. -fn toggle(me: &str, from: &Sender, ctx: &mut ServiceCtx, db: &mut dyn Store, chan: &str, setting: ChanSetting, arg: Option<&str>, name: &str) { +fn toggle(me: &str, from: &Sender, ctx: &mut ServiceCtx, db: &mut dyn Store, chan: &str, setting: ChanSetting, arg: Option<&str>) { + let name = label(setting); let on = match arg.map(|s| s.to_ascii_uppercase()).as_deref() { Some("ON") => true, Some("OFF") => false, diff --git a/inspircd/src/lib.rs b/inspircd/src/lib.rs index 395bb7d..b8e7b99 100644 --- a/inspircd/src/lib.rs +++ b/inspircd/src/lib.rs @@ -7,17 +7,18 @@ use std::time::{SystemTime, UNIX_EPOCH}; pub struct InspIrcd { sid: String, name: String, + description: String, password: String, protocol: u32, ts: u64, } impl InspIrcd { - pub fn new(name: String, sid: String, password: String, protocol: u32, ts: u64) -> Self { - Self { sid, name, password, protocol, ts } + pub fn new(name: String, description: String, sid: String, password: String, protocol: u32, ts: u64) -> Self { + Self { sid, name, description, password, protocol, ts } } - fn from_us(&self, cmd: String) -> String { + fn sourced(&self, cmd: String) -> String { format!(":{} {}", self.sid, cmd) } } @@ -28,7 +29,7 @@ impl Protocol for InspIrcd { format!("CAPAB START {}", self.protocol), format!("CAPAB CAPABILITIES :PROTOCOL={}", self.protocol), "CAPAB END".to_string(), - format!("SERVER {} {} {} :{}", self.name, self.password, self.sid, "Federated Services"), + format!("SERVER {} {} {} :{}", self.name, self.password, self.sid, self.description), ] } @@ -36,7 +37,7 @@ impl Protocol for InspIrcd { // Strip an optional IRCv3 message-tag prefix (@k=v;… ) — insp tags PRIVMSGs // with time/msgid, and it sits before the :source. let line = match line.strip_prefix('@') { - Some(rest) => rest.splitn(2, ' ').nth(1).unwrap_or(""), + Some(rest) => rest.split_once(' ').map(|x| x.1).unwrap_or(""), None => line, }; let (source, rest) = match line.strip_prefix(':') { @@ -227,15 +228,15 @@ impl Protocol for InspIrcd { fn serialize(&mut self, action: &NetAction) -> Vec { match action { - NetAction::Burst => vec![self.from_us(format!("BURST {}", self.ts))], - NetAction::EndBurst => vec![self.from_us("ENDBURST".to_string())], + NetAction::Burst => vec![self.sourced(format!("BURST {}", self.ts))], + NetAction::EndBurst => vec![self.sourced("ENDBURST".to_string())], NetAction::Pong { token, from } => { let dest = from.clone().unwrap_or_else(|| self.sid.clone()); - vec![self.from_us(format!("PONG {} {}", dest, token))] + vec![self.sourced(format!("PONG {} {}", dest, token))] } // insp4 UID: uuid nickts nick realhost disphost realuser dispuser ip signonts +modes :gecos // (both a real AND a displayed user field — see m_spanningtree/uid.cpp Builder). - NetAction::IntroduceUser { uid, nick, ident, host, gecos } => vec![self.from_us(format!( + NetAction::IntroduceUser { uid, nick, ident, host, gecos } => vec![self.sourced(format!( "UID {uid} {ts} {nick} {host} {host} {ident} {ident} 0.0.0.0 {ts} +i :{gecos}", uid = uid, ts = self.ts, nick = nick, host = host, ident = ident, gecos = gecos ))], @@ -247,7 +248,7 @@ impl Protocol for InspIrcd { } // ACCTREGRESULT : NetAction::AccountResponse { reqid, kind, account, status, code, message } => { - vec![self.from_us(format!( + vec![self.sourced(format!( "ACCTREGRESULT {} {} {} {} {} :{}", reqid, kind, account, status, code, message ))] @@ -259,11 +260,11 @@ impl Protocol for InspIrcd { line.push(' '); line.push_str(d); } - vec![self.from_us(line)] + vec![self.sourced(line)] } // METADATA : — "*" is server-global metadata. NetAction::Metadata { target, key, value } => { - vec![self.from_us(format!("METADATA {} {} :{}", target, key, value))] + vec![self.sourced(format!("METADATA {} {} :{}", target, key, value))] } // SVSNICK — the new nick takes the current // time as its TS so it wins any collision resolution. @@ -273,23 +274,23 @@ impl Protocol for InspIrcd { // ENCAP the target's server: CHGHOST , to set a vhost. NetAction::SetHost { uid, host } => { let target = uid.get(..3).unwrap_or(uid.as_str()); - vec![self.from_us(format!("ENCAP {} CHGHOST {} {}", target, uid, host))] + vec![self.sourced(format!("ENCAP {} CHGHOST {} {}", target, uid, host))] } NetAction::SetIdent { uid, ident } => { let target = uid.get(..3).unwrap_or(uid.as_str()); - vec![self.from_us(format!("ENCAP {} CHGIDENT {} {}", target, uid, ident))] + vec![self.sourced(format!("ENCAP {} CHGIDENT {} {}", target, uid, ident))] } NetAction::ForceNick { uid, nick } => { let now = SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(self.ts); - vec![self.from_us(format!("SVSNICK {} {} {}", uid, nick, now))] + vec![self.sourced(format!("SVSNICK {} {} {}", uid, nick, now))] } // SVSJOIN [key]: force a user into a channel, sourced from // the services server. Used to apply an account's auto-join list. NetAction::ForceJoin { uid, channel, key } => { if key.is_empty() { - vec![self.from_us(format!("SVSJOIN {} {}", uid, channel))] + vec![self.sourced(format!("SVSJOIN {} {}", uid, channel))] } else { - vec![self.from_us(format!("SVSJOIN {} {} {}", uid, channel, key))] + vec![self.sourced(format!("SVSJOIN {} {} {}", uid, channel, key))] } } // FMODE . The ircd drops an FMODE whose TS is newer @@ -299,7 +300,7 @@ impl Protocol for InspIrcd { NetAction::ChannelMode { from, channel, modes } => { let cmd = format!("FMODE {} 1 {}", channel, modes); if from.is_empty() { - vec![self.from_us(cmd)] + vec![self.sourced(cmd)] } else { vec![format!(":{} {}", from, cmd)] } @@ -397,7 +398,7 @@ mod tests { use super::*; fn proto() -> InspIrcd { - InspIrcd::new("services.test".into(), "42S".into(), "pw".into(), 1206, 1) + InspIrcd::new("services.test".into(), "Federated Services".into(), "42S".into(), "pw".into(), 1206, 1) } // A remote nick change must surface as a NickChange for the source uid, or diff --git a/nickserv/src/ghost.rs b/nickserv/src/ghost.rs index a1cc770..334e8e7 100644 --- a/nickserv/src/ghost.rs +++ b/nickserv/src/ghost.rs @@ -4,6 +4,9 @@ use fedserv_api::NetView; // GHOST/RECOVER [password]: rename off a session using a nick you own, // either by being identified to its account or giving that account's password. +// Every argument is a distinct input the guest-rename needs (the guest nick and +// its sequence counter among them), so the count is inherent. +#[allow(clippy::too_many_arguments)] pub fn handle(me: &str, guest_nick: &str, guest_seq: &mut u32, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, net: &dyn NetView, db: &dyn Store) { let Some(&target) = args.get(1) else { ctx.notice(me, from.uid, "Syntax: GHOST [password]"); diff --git a/nickserv/src/ungroup.rs b/nickserv/src/ungroup.rs index 80a3b7f..2e6860c 100644 --- a/nickserv/src/ungroup.rs +++ b/nickserv/src/ungroup.rs @@ -12,7 +12,7 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: ctx.notice(me, from.uid, "You can't ungroup your main account name."); return; } - if db.resolve_account(nick).map_or(true, |a| !a.eq_ignore_ascii_case(account)) { + if db.resolve_account(nick).is_none_or(|a| !a.eq_ignore_ascii_case(account)) { ctx.notice(me, from.uid, format!("\x02{nick}\x02 isn't grouped to your account.")); return; } diff --git a/src/engine/db.rs b/src/engine/db.rs index d93976a..371b83a 100644 --- a/src/engine/db.rs +++ b/src/engine/db.rs @@ -109,7 +109,9 @@ fn verified_default() -> bool { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "event")] pub enum Event { - AccountRegistered(Account), + // Boxed: Account is far larger than any other variant, so this keeps every + // small event in the log from being sized to it. + AccountRegistered(Box), CertAdded { account: String, fp: String }, CertRemoved { account: String, fp: String }, AccountEmailSet { account: String, email: Option }, @@ -692,7 +694,7 @@ impl EventLog { self.entries .iter() .filter(|e| e.event.scope() == Scope::Global) - .filter(|e| peer.get(&e.origin).map_or(true, |&s| e.seq > s)) + .filter(|e| peer.get(&e.origin).is_none_or(|&s| e.seq > s)) .cloned() .collect() } @@ -879,7 +881,7 @@ impl Db { /// Rewrite the log to one entry per account and channel, reclaiming churn. pub fn compact(&mut self) -> std::io::Result<()> { let before = self.log.len(); - let mut snapshot: Vec = self.accounts.values().cloned().map(Event::AccountRegistered).collect(); + let mut snapshot: Vec = self.accounts.values().cloned().map(|a| Event::AccountRegistered(Box::new(a))).collect(); for c in self.channels.values() { snapshot.push(Event::ChannelRegistered { name: c.name.clone(), founder: c.founder.clone(), ts: c.ts }); if !c.lock_on.is_empty() || !c.lock_off.is_empty() { @@ -1041,7 +1043,7 @@ impl Db { vhost: None, vhost_request: None, }; - self.log.append(Event::AccountRegistered(account.clone())).map_err(|_| RegError::Internal)?; + self.log.append(Event::AccountRegistered(Box::new(account.clone()))).map_err(|_| RegError::Internal)?; self.accounts.insert(key(name), account); Ok(()) } @@ -1081,7 +1083,7 @@ impl Db { /// globally unique, so this is unambiguous. pub fn certfp_owner(&self, fp: &str) -> Option<&str> { let fp = fp.to_ascii_lowercase(); - self.accounts.values().find(|a| a.certfps.iter().any(|c| *c == fp)).map(|a| a.name.as_str()) + self.accounts.values().find(|a| a.certfps.contains(&fp)).map(|a| a.name.as_str()) } /// The fingerprints registered to an account (empty if unknown/none). @@ -1096,7 +1098,7 @@ impl Db { /// Whether `account`'s email is confirmed (true for unknown/legacy accounts). pub fn is_verified(&self, account: &str) -> bool { - self.account(account).map_or(true, |a| a.verified) + self.account(account).is_none_or(|a| a.verified) } /// Mark `account`'s email confirmed. @@ -1443,7 +1445,7 @@ impl Db { let k = key(account); match self.accounts.get(&k) { None => return Err(CertError::NoAccount), - Some(a) if !a.certfps.iter().any(|c| *c == fp) => return Ok(false), + Some(a) if !a.certfps.contains(&fp) => return Ok(false), Some(_) => {} } self.log.append(Event::CertRemoved { account: account.to_string(), fp: fp.clone() }).map_err(|_| CertError::Internal)?; @@ -2169,7 +2171,7 @@ fn apply(accounts: &mut HashMap, channels: &mut HashMap { @@ -2826,8 +2828,8 @@ mod tests { }; let converge = |first: &Account, second: &Account| { let (mut acc, mut ch, mut gr, mut bo, mut hc) = (HashMap::new(), HashMap::new(), HashMap::new(), HashMap::new(), HostConfig::default()); - apply(&mut acc, &mut ch, &mut gr, &mut bo, &mut hc, Event::AccountRegistered(first.clone())); - apply(&mut acc, &mut ch, &mut gr, &mut bo, &mut hc, Event::AccountRegistered(second.clone())); + apply(&mut acc, &mut ch, &mut gr, &mut bo, &mut hc, Event::AccountRegistered(Box::new(first.clone()))); + apply(&mut acc, &mut ch, &mut gr, &mut bo, &mut hc, Event::AccountRegistered(Box::new(second.clone()))); acc["alice"].password_hash.clone() }; // Earlier registration wins, regardless of which claim applies first. @@ -3100,7 +3102,7 @@ mod tests { name: "bob".into(), password_hash: "x".into(), email: None, ts: 0, home: "peer".into(), scram256: None, scram512: None, certfps: vec![], verified: true, ajoin: vec![], suspension: None, memos: vec![], greet: String::new(), vhost: None, vhost_request: None, }; - let entry = LogEntry { origin: "peer".into(), seq: 0, lamport: 1, event: Event::AccountRegistered(bob) }; + let entry = LogEntry { origin: "peer".into(), seq: 0, lamport: 1, event: Event::AccountRegistered(Box::new(bob)) }; db.ingest(entry).unwrap(); assert!(db.exists("bob"), "peer's account is present locally"); assert!(db.exists("alice"), "local account still there"); diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 69f5e7c..09d295d 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -1916,7 +1916,7 @@ mod tests { name: "alice".into(), password_hash: "OTHER".into(), email: None, ts: 0, home: "peer".into(), scram256: None, scram512: None, certfps: vec![], verified: true, ajoin: vec![], suspension: None, memos: vec![], greet: String::new(), vhost: None, vhost_request: None, }; - let entry = LogEntry::for_test("peer", 0, 1, db::Event::AccountRegistered(winner)); + let entry = LogEntry::for_test("peer", 0, 1, db::Event::AccountRegistered(Box::new(winner))); e.gossip_ingest(entry).unwrap(); // The local session is logged out and its orphaned channel is dropped. diff --git a/src/gossip.rs b/src/gossip.rs index cf8143e..f2f8979 100644 --- a/src/gossip.rs +++ b/src/gossip.rs @@ -35,7 +35,9 @@ enum Msg { #[serde(default)] reply: bool, }, - Entry { entry: LogEntry }, + // Boxed: a LogEntry carries an Event, whose largest variant dwarfs the other + // Msg variants. + Entry { entry: Box }, } // Start the listener (if bound) and a dialer per configured peer. When TLS is @@ -203,7 +205,7 @@ where loop { match rx.recv().await { Ok(entry) => { - if send(&tx, &Msg::Entry { entry }).await.is_err() { + if send(&tx, &Msg::Entry { entry: Box::new(entry) }).await.is_err() { break; } } @@ -229,7 +231,7 @@ where Ok(Msg::Digest { versions, reply }) => { let missing = engine.lock().await.gossip_missing(&versions); for entry in missing { - let _ = send(&tx, &Msg::Entry { entry }).await; + let _ = send(&tx, &Msg::Entry { entry: Box::new(entry) }).await; } if reply { let versions = engine.lock().await.gossip_digest(); @@ -237,7 +239,7 @@ where } } Ok(Msg::Entry { entry }) => { - if let Err(e) = engine.lock().await.gossip_ingest(entry) { + if let Err(e) = engine.lock().await.gossip_ingest(*entry) { tracing::warn!(%e, "gossip ingest failed"); } } diff --git a/src/grpc.rs b/src/grpc.rs index 0889f89..057f945 100644 --- a/src/grpc.rs +++ b/src/grpc.rs @@ -42,6 +42,9 @@ const SUBSCRIBER_BUFFER: usize = 1024; // Every RPC (both services) needs `authorization: Bearer ` matching the // configured secret. Constant-time compare — same posture as password/secret // checks elsewhere in this codebase, cheap insurance against a timing side-channel. +// Status is tonic's error type, used by every RPC — boxing it here would fight +// the framework's convention for marginal size gain. +#[allow(clippy::result_large_err)] fn authorize(req: &Request, token: &str) -> Result<(), Status> { let got = req .metadata() @@ -409,7 +412,7 @@ mod tests { vhost: None, vhost_request: None, }; - let registered = LogEntry::for_test("A", 0, 1, Event::AccountRegistered(acct)); + let registered = LogEntry::for_test("A", 0, 1, Event::AccountRegistered(Box::new(acct))); let wire = to_wire(®istered).expect("account registration replicates"); match wire.kind { Some(Kind::AccountRegistered(a)) => { diff --git a/src/main.rs b/src/main.rs index 6d07e51..c8641f8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -39,6 +39,7 @@ async fn main() -> Result<()> { let proto = Box::new(InspIrcd::new( cfg.server.name.clone(), + cfg.server.description.clone(), cfg.server.sid.clone(), cfg.uplink.password.clone(), cfg.server.protocol,