Skip to content

Commit f17dda8

Browse files
authored
Couple of minor cleanups (#970)
2 parents 7779976 + 737533c commit f17dda8

File tree

3 files changed

+22
-45
lines changed

3 files changed

+22
-45
lines changed

src/exec/event.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{cutils::cerr, log::dev_debug};
1111

1212
pub(super) trait Process: Sized {
1313
/// IO Events that this process should handle.
14-
type Event: Copy + Eq + Debug;
14+
type Event: Copy + Debug;
1515
/// Reason why the event loop should break.
1616
///
1717
/// See [`EventRegistry::set_break`] for more information.
@@ -208,8 +208,7 @@ impl<T: Process> EventRegistry<T> {
208208
self.status = Status::Stop(StopReason::Exit(reason));
209209
}
210210

211-
/// Return whether a break reason has been set already. This function will return `false` after
212-
/// [`EventRegistry::event_loop`] has been called.
211+
/// Return whether a break reason has been set already.
213212
pub(super) fn got_break(&self) -> bool {
214213
self.status.is_break()
215214
}
@@ -219,7 +218,7 @@ impl<T: Process> EventRegistry<T> {
219218
/// The event loop will continue indefinitely unless you call [`EventRegistry::set_break`] or
220219
/// [`EventRegistry::set_exit`].
221220
#[track_caller]
222-
pub(super) fn event_loop(&mut self, process: &mut T) -> StopReason<T> {
221+
pub(super) fn event_loop(mut self, process: &mut T) -> StopReason<T> {
223222
let mut event_queue = Vec::with_capacity(self.poll_fds.len());
224223

225224
loop {
@@ -232,7 +231,7 @@ impl<T: Process> EventRegistry<T> {
232231
}
233232

234233
for event in event_queue.drain(..) {
235-
process.on_event(event, self);
234+
process.on_event(event, &mut self);
236235

237236
if let Some(reason) = self.status.take_exit() {
238237
return StopReason::Exit(reason);

src/exec/use_pty/parent.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ pub(in crate::exec) fn exec_pty(
235235
}
236236
}
237237

238-
let exit_reason = closure.run(&mut registry);
238+
let exit_reason = closure.run(registry);
239239
// FIXME (ogsudo): Retry if `/dev/tty` is revoked.
240240

241241
// Flush the terminal
@@ -350,7 +350,7 @@ impl ParentClosure {
350350
})
351351
}
352352

353-
fn run(&mut self, registry: &mut EventRegistry<Self>) -> io::Result<ExitReason> {
353+
fn run(&mut self, registry: EventRegistry<Self>) -> io::Result<ExitReason> {
354354
match registry.event_loop(self) {
355355
StopReason::Break(err) | StopReason::Exit(ParentExit::Backchannel(err)) => Err(err),
356356
StopReason::Exit(ParentExit::Command(exit_reason)) => Ok(exit_reason),

src/su/cli.rs

Lines changed: 16 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{borrow::Cow, mem, path::PathBuf};
1+
use std::{mem, path::PathBuf};
22

33
use crate::common::SudoString;
44

@@ -138,20 +138,20 @@ impl TryFrom<SuOptions> for SuRunOptions {
138138
}
139139

140140
fn reject_all(context: &str, opts: SuOptions) -> Result<(), String> {
141-
macro_rules! tuple {
142-
($expr:expr) => {
143-
(&$expr as &dyn IsAbsent, {
144-
let name = concat!("--", stringify!($expr));
145-
if name.contains('_') {
146-
Cow::Owned(name.replace('_', "-"))
147-
} else {
148-
Cow::Borrowed(name)
149-
}
150-
})
141+
macro_rules! ensure_options_absent {
142+
($($opt:ident,)*) => {
143+
let SuOptions {
144+
$($opt),*
145+
} = opts;
146+
147+
$(if !$opt.is_absent() {
148+
let name = concat!("--", stringify!($opt)).replace('_', "-");
149+
return Err(format!("{context} conflicts with {name}"));
150+
})*
151151
};
152152
}
153153

154-
let SuOptions {
154+
ensure_options_absent! {
155155
command,
156156
group,
157157
help,
@@ -163,37 +163,15 @@ fn reject_all(context: &str, opts: SuOptions) -> Result<(), String> {
163163
version,
164164
whitelist_environment,
165165
positional_args,
166-
} = opts;
167-
168-
let flags = [
169-
tuple!(command),
170-
tuple!(group),
171-
tuple!(help),
172-
tuple!(login),
173-
tuple!(preserve_environment),
174-
tuple!(pty),
175-
tuple!(shell),
176-
tuple!(supp_group),
177-
tuple!(version),
178-
tuple!(whitelist_environment),
179-
];
180-
for (value, name) in flags {
181-
ensure_is_absent(context, value, &name)?;
182-
}
166+
};
183167

184-
ensure_is_absent(context, &positional_args, "positional argument")?;
168+
if !positional_args.is_absent() {
169+
return Err(format!("{context} conflicts with positional argument"));
170+
}
185171

186172
Ok(())
187173
}
188174

189-
fn ensure_is_absent(context: &str, thing: &dyn IsAbsent, name: &str) -> Result<(), String> {
190-
if thing.is_absent() {
191-
Ok(())
192-
} else {
193-
Err(format!("{context} conflicts with {name}"))
194-
}
195-
}
196-
197175
trait IsAbsent {
198176
fn is_absent(&self) -> bool;
199177
}

0 commit comments

Comments
 (0)