diff --git a/src/exec/no_pty.rs b/src/exec/no_pty.rs index 93175b7cc..b7c8e2019 100644 --- a/src/exec/no_pty.rs +++ b/src/exec/no_pty.rs @@ -14,7 +14,7 @@ use crate::{ }, }; use crate::{ - exec::{handle_sigchld, opt_fmt, signal_fmt}, + exec::{handle_sigchld, signal_fmt}, log::{dev_error, dev_info, dev_warn}, system::{ fork, getpgid, getpgrp, @@ -238,12 +238,7 @@ impl ExecClosure { } }; - dev_info!( - "received{} {} from {}", - opt_fmt(info.is_user_signaled(), " user signaled"), - info.signal(), - info.pid() - ); + dev_info!("received{}", info); let Some(command_pid) = self.command_pid else { dev_info!("command was terminated, ignoring signal"); @@ -256,9 +251,11 @@ impl ExecClosure { // FIXME: we should handle SIGWINCH here if we want to support I/O plugins that // react on window change events. - // Skip the signal if it was sent by the user and it is self-terminating. - if info.is_user_signaled() && self.is_self_terminating(info.pid()) { - return; + if let Some(pid) = info.signaler_pid() { + if self.is_self_terminating(pid) { + // Skip the signal if it was sent by the user and it is self-terminating. + return; + } } if signal == SIGALRM { diff --git a/src/exec/use_pty/monitor.rs b/src/exec/use_pty/monitor.rs index bb321db43..20e06226c 100644 --- a/src/exec/use_pty/monitor.rs +++ b/src/exec/use_pty/monitor.rs @@ -381,12 +381,7 @@ impl<'a> MonitorClosure<'a> { } }; - dev_info!( - "monitor received{} {} from {}", - opt_fmt(info.is_user_signaled(), " user signaled"), - info.signal(), - info.pid() - ); + dev_info!("monitor received{}", info); // Don't do anything if the command has terminated already let Some(command_pid) = self.command_pid else { @@ -396,10 +391,16 @@ impl<'a> MonitorClosure<'a> { match info.signal() { SIGCHLD => handle_sigchld(self, registry, "command", command_pid), - // Skip the signal if it was sent by the user and it is self-terminating. - _ if info.is_user_signaled() - && is_self_terminating(info.pid(), command_pid, self.command_pgrp) => {} - signal => self.send_signal(signal, command_pid, false), + signal => { + if let Some(pid) = info.signaler_pid() { + if is_self_terminating(pid, command_pid, self.command_pgrp) { + // Skip the signal if it was sent by the user and it is self-terminating. + return; + } + } + + self.send_signal(signal, command_pid, false) + } } } } diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index 37e530df6..aed7b9dbb 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -7,7 +7,7 @@ use crate::exec::event::{EventHandle, EventRegistry, PollEvent, Process, StopRea use crate::exec::use_pty::monitor::exec_monitor; use crate::exec::use_pty::SIGCONT_FG; use crate::exec::{ - cond_fmt, handle_sigchld, opt_fmt, signal_fmt, terminate_process, ExecOutput, HandleSigchld, + cond_fmt, handle_sigchld, signal_fmt, terminate_process, ExecOutput, HandleSigchld, ProcessOutput, }; use crate::exec::{ @@ -617,12 +617,7 @@ impl ParentClosure { } }; - dev_info!( - "parent received{} {} from {}", - opt_fmt(info.is_user_signaled(), " user signaled"), - info.signal(), - info.pid() - ); + dev_info!("parent received{}", info); let Some(monitor_pid) = self.monitor_pid else { dev_info!("monitor was terminated, ignoring signal"); @@ -639,10 +634,17 @@ impl ParentClosure { dev_warn!("cannot resize terminal: {}", err); } } - // Skip the signal if it was sent by the user and it is self-terminating. - _ if info.is_user_signaled() && self.is_self_terminating(info.pid()) => {} - // FIXME: check `send_command_status` - signal => self.schedule_signal(signal, registry), + signal => { + if let Some(pid) = info.signaler_pid() { + if self.is_self_terminating(pid) { + // Skip the signal if it was sent by the user and it is self-terminating. + return; + } + } + + // FIXME: check `send_command_status` + self.schedule_signal(signal, registry) + } } } diff --git a/src/system/signal/info.rs b/src/system/signal/info.rs index 31c31dbd2..bc6d44793 100644 --- a/src/system/signal/info.rs +++ b/src/system/signal/info.rs @@ -1,3 +1,5 @@ +use std::fmt; + use crate::system::interface::ProcessId; use super::SignalNumber; @@ -12,16 +14,19 @@ impl SignalInfo { pub(super) const SIZE: usize = std::mem::size_of::(); /// Returns whether the signal was sent by the user or not. - pub(crate) fn is_user_signaled(&self) -> bool { - // FIXME: we should check if si_code is equal to SI_USER but for some reason the latter it - // is not available in libc. + fn is_user_signaled(&self) -> bool { + // This matches the definition of the SI_FROMUSER macro. self.info.si_code <= 0 } /// Gets the PID that sent the signal. - pub(crate) fn pid(&self) -> ProcessId { - // FIXME: some signals don't set si_pid. - unsafe { self.info.si_pid() } + pub(crate) fn signaler_pid(&self) -> Option { + if self.is_user_signaled() { + // SAFETY: si_pid is always initialized if the signal is user signaled. + unsafe { Some(self.info.si_pid()) } + } else { + None + } } /// Gets the signal number. @@ -29,3 +34,23 @@ impl SignalInfo { self.info.si_signo } } + +impl fmt::Display for SignalInfo { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{} {} from ", + if self.is_user_signaled() { + " user signaled" + } else { + "" + }, + self.signal(), + )?; + if let Some(pid) = self.signaler_pid() { + write!(f, "{pid}") + } else { + write!(f, "") + } + } +} diff --git a/src/system/signal/set.rs b/src/system/signal/set.rs index c566b2c7c..3356bc73d 100644 --- a/src/system/signal/set.rs +++ b/src/system/signal/set.rs @@ -41,8 +41,11 @@ impl SignalAction { pub(super) fn register(&self, signal: SignalNumber) -> io::Result { let mut original_action = MaybeUninit::::zeroed(); + // SAFETY: `sigaction` expects a valid pointer, which we provide; the typecast is valid + // since SignalAction is a repr(transparent) newtype struct. cerr(unsafe { libc::sigaction(signal, &self.raw, original_action.as_mut_ptr().cast()) })?; + // SAFETY: `sigaction` will have properly initialized `original_action`. Ok(unsafe { original_action.assume_init() }) } } @@ -58,8 +61,10 @@ impl SignalSet { pub(crate) fn empty() -> io::Result { let mut set = MaybeUninit::::zeroed(); + // SAFETY: same as above cerr(unsafe { libc::sigemptyset(set.as_mut_ptr().cast()) })?; + // SAFETY: `sigemptyset` will have initialized `set` Ok(unsafe { set.assume_init() }) } @@ -67,16 +72,20 @@ impl SignalSet { pub(crate) fn full() -> io::Result { let mut set = MaybeUninit::::zeroed(); + // SAFETY: same as above cerr(unsafe { libc::sigfillset(set.as_mut_ptr().cast()) })?; + // SAFETY: `sigfillset` will have initialized `set` Ok(unsafe { set.assume_init() }) } fn sigprocmask(&self, how: libc::c_int) -> io::Result { let mut original_set = MaybeUninit::::zeroed(); + // SAFETY: same as above cerr(unsafe { libc::sigprocmask(how, &self.raw, original_set.as_mut_ptr().cast()) })?; + // SAFETY: `sigprocmask` will have initialized `set` Ok(unsafe { original_set.assume_init() }) } diff --git a/src/system/signal/stream.rs b/src/system/signal/stream.rs index f9236f3a6..abaf4e9a6 100644 --- a/src/system/signal/stream.rs +++ b/src/system/signal/stream.rs @@ -67,6 +67,9 @@ impl SignalStream { pub(crate) fn recv(&self) -> io::Result { let mut info = MaybeUninit::::uninit(); let fd = self.rx.as_raw_fd(); + // SAFETY: type invariant for `SignalStream` ensures that `fd` is a valid file descriptor; + // furthermore, `info` is a valid pointer to `siginfo_t` (by virtue of `SignalInfo` being a + // transparent newtype for it), which has room for `SignalInfo::SIZE` bytes. let bytes = cerr(unsafe { libc::recv(fd, info.as_mut_ptr().cast(), SignalInfo::SIZE, 0) })?; if bytes as usize != SignalInfo::SIZE { @@ -97,6 +100,8 @@ pub(crate) fn register_handlers( })?; } + // SAFETY: if the above for-loop has terminated, every handler will have + // been written to via "MaybeUnit::new", and so is initialized. Ok(handlers.map(|(_, handler)| unsafe { handler.assume_init() })) } diff --git a/src/system/term/mod.rs b/src/system/term/mod.rs index 951f73c27..ae0bba5d7 100644 --- a/src/system/term/mod.rs +++ b/src/system/term/mod.rs @@ -34,6 +34,11 @@ impl Pty { // Create two integers to hold the file descriptors for each side of the pty. let (mut leader, mut follower) = (0, 0); + // SAFETY: + // - openpty is passed two valid pointers as its first two arguments + // - path is a valid array that can hold PATH_MAX characters; and casting `u8` to `i8` is + // valid since all values are initialized to zero. + // - the last two arguments are allowed to be NULL cerr(unsafe { libc::openpty( &mut leader, @@ -60,9 +65,11 @@ impl Pty { Ok(Self { path, leader: PtyLeader { + // SAFETY: `openpty` has set `leader` to an open fd suitable for assuming ownership by `OwnedFd`. file: unsafe { OwnedFd::from_raw_fd(leader) }.into(), }, follower: PtyFollower { + // SAFETY: `openpty` has set `follower` to an open fd suitable for assuming ownership by `OwnedFd`. file: unsafe { OwnedFd::from_raw_fd(follower) }.into(), }, }) @@ -75,6 +82,11 @@ pub(crate) struct PtyLeader { impl PtyLeader { pub(crate) fn set_size(&self, term_size: &TermSize) -> io::Result<()> { + // SAFETY: the TIOCSWINSZ expects an initialized pointer of type `winsize` + // https://www.man7.org/linux/man-pages/man2/TIOCSWINSZ.2const.html + // + // An object of type TermSize is safe to cast to `winsize` since it is a + // repr(transparent) "newtype" struct. cerr(unsafe { ioctl( self.file.as_raw_fd(), @@ -151,15 +163,19 @@ pub(crate) trait Terminal: sealed::Sealed { impl Terminal for F { /// Get the foreground process group ID associated with this terminal. fn tcgetpgrp(&self) -> io::Result { + // SAFETY: tcgetpgrp cannot cause UB cerr(unsafe { libc::tcgetpgrp(self.as_raw_fd()) }) } - /// Set the foreground process group ID associated with this terminalto `pgrp`. + /// Set the foreground process group ID associated with this terminal to `pgrp`. fn tcsetpgrp(&self, pgrp: ProcessId) -> io::Result<()> { + // SAFETY: tcsetpgrp cannot cause UB cerr(unsafe { libc::tcsetpgrp(self.as_raw_fd(), pgrp) }).map(|_| ()) } /// Make the given terminal the controlling terminal of the calling process. fn make_controlling_terminal(&self) -> io::Result<()> { + // SAFETY: this is a correct way to call the TIOCSCTTY ioctl, see: + // https://www.man7.org/linux/man-pages/man2/TIOCNOTTY.2const.html cerr(unsafe { libc::ioctl(self.as_raw_fd(), libc::TIOCSCTTY, 0) })?; Ok(()) } @@ -172,7 +188,9 @@ impl Terminal for F { return Err(io::ErrorKind::Unsupported.into()); } - cerr(unsafe { libc::ttyname_r(self.as_raw_fd(), buf.as_mut_ptr() as _, buf.len()) })?; + // SAFETY: `buf` is a valid and initialized pointer, and its correct length is passed + cerr(unsafe { libc::ttyname_r(self.as_raw_fd(), buf.as_mut_ptr(), buf.len()) })?; + // SAFETY: `buf` will have been initialized by the `ttyname_r` call, if it succeeded Ok(unsafe { os_string_from_ptr(buf.as_ptr()) }) } @@ -182,6 +200,7 @@ impl Terminal for F { } fn tcgetsid(&self) -> io::Result { + // SAFETY: tcgetsid cannot cause UB cerr(unsafe { libc::tcgetsid(self.as_raw_fd()) }) } } diff --git a/src/system/term/user_term.rs b/src/system/term/user_term.rs index 7e5b4446a..86758c790 100644 --- a/src/system/term/user_term.rs +++ b/src/system/term/user_term.rs @@ -56,16 +56,24 @@ const LOCAL_FLAGS: tcflag_t = ISIG static GOT_SIGTTOU: AtomicBool = AtomicBool::new(false); -extern "C" fn on_sigttou(_signal: c_int, _info: *mut siginfo_t, _: *mut c_void) { - GOT_SIGTTOU.store(true, Ordering::SeqCst); -} - -/// This is like `tcsetattr` but it only suceeds if we are in the foreground process group. -fn tcsetattr_nobg(fd: c_int, flags: c_int, tp: *const termios) -> io::Result<()> { +/// This is like `tcsetattr` but it only succeeds if we are in the foreground process group. +/// # Safety +/// +/// The arguments to this function have to be valid arguments to `tcsetattr`. +unsafe fn tcsetattr_nobg(fd: c_int, flags: c_int, tp: *const termios) -> io::Result<()> { // This function is based around the fact that we receive `SIGTTOU` if we call `tcsetattr` and // we are not in the foreground process group. - let mut original_action = MaybeUninit::::uninit(); + // SAFETY: is the responsibility of the caller of `tcsetattr_nobg` + let setattr = || cerr(unsafe { tcsetattr(fd, flags, tp) }).map(|_| ()); + + catching_sigttou(setattr) +} + +fn catching_sigttou(mut function: impl FnMut() -> io::Result<()>) -> io::Result<()> { + extern "C" fn on_sigttou(_signal: c_int, _info: *mut siginfo_t, _: *mut c_void) { + GOT_SIGTTOU.store(true, Ordering::SeqCst); + } let action = { let mut raw: libc::sigaction = make_zeroed_sigaction(); @@ -74,20 +82,37 @@ fn tcsetattr_nobg(fd: c_int, flags: c_int, tp: *const termios) -> io::Result<()> // Exclude any other signals from the set raw.sa_mask = { let mut sa_mask = MaybeUninit::::uninit(); - unsafe { sigemptyset(sa_mask.as_mut_ptr()) }; - unsafe { sa_mask.assume_init() } + // SAFETY: sa_mask is a valid and dereferenceble pointer; it will + // become initialized by `sigemptyset` + unsafe { + sigemptyset(sa_mask.as_mut_ptr()); + sa_mask.assume_init() + } }; raw.sa_flags = 0; raw }; // Reset `GOT_SIGTTOU`. GOT_SIGTTOU.store(false, Ordering::SeqCst); + // Set `action` as the action for `SIGTTOU` and store the original action in `original_action` // to restore it later. - unsafe { sigaction(SIGTTOU, &action, original_action.as_mut_ptr()) }; + // + // SAFETY: `original_action` is a valid pointer; second, the `action` installed (on_sigttou): + // - is itself a safe function + // - only updates an atomic variable, so cannot violate memory unsafety that way + // - doesn't call any async-unsafe functions (refer to signal-safety(7)) + // Therefore it can safely be installed as a signal handler. + // Furthermore, `sigaction` will initialize `original_action`. + let original_action = unsafe { + let mut original_action = MaybeUninit::::uninit(); + sigaction(SIGTTOU, &action, original_action.as_mut_ptr()); + original_action.assume_init() + }; + // Call `tcsetattr` until it suceeds and ignore interruptions if we did not receive `SIGTTOU`. let result = loop { - match cerr(unsafe { tcsetattr(fd, flags, tp) }) { + match function() { Ok(_) => break Ok(()), Err(err) => { let got_sigttou = GOT_SIGTTOU.load(Ordering::SeqCst); @@ -97,8 +122,13 @@ fn tcsetattr_nobg(fd: c_int, flags: c_int, tp: *const termios) -> io::Result<()> } } }; + // Restore the original action. - unsafe { sigaction(SIGTTOU, original_action.as_ptr(), std::ptr::null_mut()) }; + // + // SAFETY: `original_action` is a valid pointer, and was initialized by the preceding + // call to `sigaction` (and not subsequently altered, since it is not mut). The third parameter + // is allowed to be NULL (this means we ignore the previously-installed handler) + unsafe { sigaction(SIGTTOU, &original_action, std::ptr::null_mut()) }; result } @@ -106,8 +136,7 @@ fn tcsetattr_nobg(fd: c_int, flags: c_int, tp: *const termios) -> io::Result<()> /// Type to manipulate the settings of the user's terminal. pub struct UserTerm { tty: File, - original_termios: MaybeUninit, - changed: bool, + original_termios: Option, } impl UserTerm { @@ -115,14 +144,16 @@ impl UserTerm { pub fn open() -> io::Result { Ok(Self { tty: OpenOptions::new().read(true).write(true).open("/dev/tty")?, - original_termios: MaybeUninit::uninit(), - changed: false, + original_termios: None, }) } pub(crate) fn get_size(&self) -> io::Result { let mut term_size = MaybeUninit::::uninit(); + // SAFETY: This passes a valid file descriptor and valid pointer (of + // the correct type) to the TIOCGWINSZ ioctl; see: + // https://man7.org/linux/man-pages/man2/TIOCGWINSZ.2const.html cerr(unsafe { ioctl( self.tty.as_raw_fd(), @@ -131,6 +162,7 @@ impl UserTerm { ) })?; + // SAFETY: if we arrived at this point, `term_size` was initialized. Ok(unsafe { term_size.assume_init() }) } @@ -139,15 +171,16 @@ impl UserTerm { let src = self.tty.as_raw_fd(); let dst = dst.as_raw_fd(); - let mut tt_src = MaybeUninit::::uninit(); - let mut tt_dst = MaybeUninit::::uninit(); - let mut wsize = MaybeUninit::::uninit(); + // SAFETY: tt_src and tt_dst will be initialized by `tcgetattr`. + let (tt_src, mut tt_dst) = unsafe { + let mut tt_src = MaybeUninit::::uninit(); + let mut tt_dst = MaybeUninit::::uninit(); - cerr(unsafe { tcgetattr(src, tt_src.as_mut_ptr()) })?; - cerr(unsafe { tcgetattr(dst, tt_dst.as_mut_ptr()) })?; + cerr(tcgetattr(src, tt_src.as_mut_ptr()))?; + cerr(tcgetattr(dst, tt_dst.as_mut_ptr()))?; - let tt_src = unsafe { tt_src.assume_init() }; - let mut tt_dst = unsafe { tt_dst.assume_init() }; + (tt_src.assume_init(), tt_dst.assume_init()) + }; // Clear select input, output, control and local flags. tt_dst.c_iflag &= !INPUT_FLAGS; @@ -165,21 +198,31 @@ impl UserTerm { tt_dst.c_cc.copy_from_slice(&tt_src.c_cc); // Copy speed from `src`. - { - let mut speed = unsafe { cfgetospeed(&tt_src) }; + // + // SAFETY: the cfXXXXspeed calls are passed valid pointers and + // cannot cause UB even if the speed would be incorrect. + unsafe { + let mut speed = cfgetospeed(&tt_src); // Zero output speed closes the connection. if speed == libc::B0 { speed = libc::B38400; } - unsafe { cfsetospeed(&mut tt_dst, speed) }; - speed = unsafe { cfgetispeed(&tt_src) }; - unsafe { cfsetispeed(&mut tt_dst, speed) }; + cfsetospeed(&mut tt_dst, speed); + + speed = cfgetispeed(&tt_src); + cfsetispeed(&mut tt_dst, speed); } - tcsetattr_nobg(dst, TCSAFLUSH, &tt_dst)?; + // SAFETY: dst is a valid file descriptor and `tt_dst` is an + // initialized struct obtained through tcgetattr; so this is safe to + // pass to `tcsetattr`. + unsafe { tcsetattr_nobg(dst, TCSAFLUSH, &tt_dst) }?; - cerr(unsafe { ioctl(src, TIOCGWINSZ, &mut wsize) })?; - cerr(unsafe { ioctl(dst, TIOCSWINSZ, &wsize) })?; + let mut wsize = MaybeUninit::::uninit(); + // SAFETY: TIOCGWINSZ ioctl expects one argument of type *mut winsize + cerr(unsafe { ioctl(src, TIOCGWINSZ, wsize.as_mut_ptr()) })?; + // SAFETY: wsize has been initialized by the TIOCGWINSZ ioctl + cerr(unsafe { ioctl(dst, TIOCSWINSZ, wsize.as_ptr()) })?; Ok(()) } @@ -189,20 +232,30 @@ impl UserTerm { pub fn set_raw_mode(&mut self, with_signals: bool) -> io::Result<()> { let fd = self.tty.as_raw_fd(); - if !self.changed { - cerr(unsafe { tcgetattr(fd, self.original_termios.as_mut_ptr()) })?; - } - // Retrieve the original terminal. - let mut term = unsafe { self.original_termios.assume_init() }; + // Retrieve the original terminal (if we haven't done so already) + let mut term = if let Some(termios) = self.original_termios { + termios + } else { + // SAFETY: `termios` is a valid pointer to pass to tcgetattr; if that calls succeeds, + // it will have initialized the `termios` structure + *self.original_termios.insert(unsafe { + let mut termios = MaybeUninit::uninit(); + cerr(tcgetattr(fd, termios.as_mut_ptr()))?; + termios.assume_init() + }) + }; + // Set terminal to raw mode. + // SAFETY: `term` is a valid, initialized struct of type `termios`, which + // was previously obtained through `tcgetattr`. unsafe { cfmakeraw(&mut term) }; // Enable terminal signals. if with_signals { term.c_cflag |= ISIG; } - tcsetattr_nobg(fd, TCSADRAIN, &term)?; - self.changed = true; + // SAFETY: `fd` is a valid file descriptor for the tty; for `term`: same as above. + unsafe { tcsetattr_nobg(fd, TCSADRAIN, &term) }?; Ok(()) } @@ -212,11 +265,12 @@ impl UserTerm { /// This change is done after waiting for all the queued output to be written. To discard the /// queued input `flush` must be set to `true`. pub fn restore(&mut self, flush: bool) -> io::Result<()> { - if self.changed { + if let Some(termios) = self.original_termios.take() { let fd = self.tty.as_raw_fd(); let flags = if flush { TCSAFLUSH } else { TCSADRAIN }; - tcsetattr_nobg(fd, flags, self.original_termios.as_ptr())?; - self.changed = false; + // SAFETY: `fd` is a valid file descriptor for the tty; and `termios` is a valid pointer + // that was obtained through `tcgetattr`. + unsafe { tcsetattr_nobg(fd, flags, &termios) }?; } Ok(()) @@ -227,42 +281,7 @@ impl UserTerm { // This function is based around the fact that we receive `SIGTTOU` if we call `tcsetpgrp` and // we are not in the foreground process group. - let mut original_action = MaybeUninit::::uninit(); - - let action = { - let mut raw: libc::sigaction = make_zeroed_sigaction(); - // Call `on_sigttou` if `SIGTTOU` arrives. - raw.sa_sigaction = on_sigttou as sighandler_t; - // Exclude any other signals from the set - raw.sa_mask = { - let mut sa_mask = MaybeUninit::::uninit(); - unsafe { sigemptyset(sa_mask.as_mut_ptr()) }; - unsafe { sa_mask.assume_init() } - }; - raw.sa_flags = 0; - raw - }; - // Reset `GOT_SIGTTOU`. - GOT_SIGTTOU.store(false, Ordering::SeqCst); - // Set `action` as the action for `SIGTTOU` and store the original action in `original_action` - // to restore it later. - unsafe { sigaction(SIGTTOU, &action, original_action.as_mut_ptr()) }; - // Call `tcsetattr` until it suceeds and ignore interruptions if we did not receive `SIGTTOU`. - let result = loop { - match self.tty.tcsetpgrp(pgrp) { - Ok(()) => break Ok(()), - Err(err) => { - let got_sigttou = GOT_SIGTTOU.load(Ordering::SeqCst); - if got_sigttou || err.kind() != io::ErrorKind::Interrupted { - break Err(err); - } - } - } - }; - // Restore the original action. - unsafe { sigaction(SIGTTOU, original_action.as_ptr(), std::ptr::null_mut()) }; - - result + catching_sigttou(|| self.tty.tcsetpgrp(pgrp)) } }