Minor fixes to QCatchSyscalls support

- Refactor syscall number parsing code.
- Add support returning error codes from catch syscall handlers

examples/armv4t/gdb/catch_syscalls.rs

@@ -10,11 +10,11 @@ impl target::ext::catch_syscalls::CatchSyscalls for Emu {
     fn enable_catch_syscalls(
         &mut self,
         _filter: Option<SyscallNumbers<u32>>,
-    ) -> Result<(), Self::Error> {
+    ) -> target::TargetResult<(), Self> {
         Ok(())
     }
 
-    fn disable_catch_syscalls(&mut self) -> Result<(), Self::Error> {
+    fn disable_catch_syscalls(&mut self) -> target::TargetResult<(), Self> {
         Ok(())
     }
 }

src/gdbstub_impl/ext/catch_syscalls.rs

@@ -22,20 +22,24 @@ impl<T: Target, C: Connection> GdbStubImpl<T, C> {
         let handler_status = match command {
             CatchSyscalls::QCatchSyscalls(cmd) => {
                 match cmd {
-                    QCatchSyscalls::Disable => {
-                        ops.disable_catch_syscalls().map_err(Error::TargetError)?
-                    }
+                    QCatchSyscalls::Disable => ops.disable_catch_syscalls().handle_error()?,
                     QCatchSyscalls::Enable(sysno) => {
-                        // FIXME: report integer overflow instead of silently ignoring
+                        let mut error = false;
                         let mut filter = sysno
                             .into_iter()
-                            .filter_map(|x| <T::Arch as Arch>::Usize::from_be_bytes(x));
+                            .map(|x| <T::Arch as Arch>::Usize::from_be_bytes(x))
+                            .take_while(|x| {
+                                error = x.is_none();
+                                !error
+                            })
+                            .flatten();
                         ops.enable_catch_syscalls(Some(SyscallNumbers { inner: &mut filter }))
-                            .map_err(Error::TargetError)?
+                            .handle_error()?;
+                        if error {
+                            return Err(Error::TargetMismatch);
+                        }
                     }
-                    QCatchSyscalls::EnableAll => ops
-                        .enable_catch_syscalls(None)
-                        .map_err(Error::TargetError)?,
+                    QCatchSyscalls::EnableAll => ops.enable_catch_syscalls(None).handle_error()?,
                 }
                 HandlerStatus::NeedsOk
             }

src/protocol/commands.rs

@@ -6,7 +6,8 @@ use crate::target::Target;
 pub(self) mod prelude {
     pub use super::ParseCommand;
     pub use crate::common::*;
-    pub use crate::protocol::common::hex::{decode_hex, decode_hex_buf, is_hex, Args, HexString};
+    pub use crate::protocol::common::hex::{decode_hex, decode_hex_buf, is_hex, HexString};
+    pub use crate::protocol::common::lists;
     pub use crate::protocol::common::thread_id::{
         IdKind, SpecificIdKind, SpecificThreadId, ThreadId,
     };

src/protocol/commands/_QCatchSyscalls.rs

@@ -3,7 +3,7 @@ use super::prelude::*;
 #[derive(Debug)]
 pub enum QCatchSyscalls<'a> {
     Disable,
-    Enable(Args<'a>),
+    Enable(lists::ArgListHex<'a>),
     EnableAll,
 }
 
@@ -13,9 +13,9 @@ impl<'a> ParseCommand<'a> for QCatchSyscalls<'a> {
 
         match body {
             [b':', b'0'] => Some(QCatchSyscalls::Disable),
-            [b':', b'1', b';', sysno @ ..] => {
-                Some(QCatchSyscalls::Enable(Args::from_packet(sysno)?))
-            }
+            [b':', b'1', b';', sysno @ ..] => Some(QCatchSyscalls::Enable(
+                lists::ArgListHex::from_packet(sysno)?,
+            )),
             [b':', b'1'] => Some(QCatchSyscalls::EnableAll),
             _ => None,
         }

src/protocol/commands/_vRun.rs

@@ -3,7 +3,7 @@ use super::prelude::*;
 #[derive(Debug)]
 pub struct vRun<'a> {
     pub filename: Option<&'a [u8]>,
-    pub args: Args<'a>,
+    pub args: lists::ArgListHex<'a>,
 }
 
 impl<'a> ParseCommand<'a> for vRun<'a> {
@@ -21,7 +21,7 @@ impl<'a> ParseCommand<'a> for vRun<'a> {
 
         Some(vRun {
             filename,
-            args: Args::from_packet(args)?,
+            args: lists::ArgListHex::from_packet(args)?,
         })
     }
 }

src/protocol/common/hex.rs

@@ -143,31 +143,6 @@ pub fn encode_hex_buf(buf: &mut [u8], start_idx: usize) -> Result<&mut [u8], Enc
     Ok(&mut buf[..encoded_len])
 }
 
-/// A wrapper type around a list of hex encoded arguments separated by `;`.
-#[derive(Debug)]
-pub struct Args<'a>(&'a mut [u8]);
-
-impl<'a> Args<'a> {
-    pub fn from_packet(args: &'a mut [u8]) -> Option<Self> {
-        // validate that args have valid hex encoding (with ';' delimiters).
-        // this removes all the error handling from the lazy `Args` iterator.
-        if args.iter().any(|b| !(is_hex(*b) || *b == b';')) {
-            return None;
-        }
-        Some(Self(args))
-    }
-
-    pub fn into_iter(self) -> impl Iterator<Item = &'a [u8]> + 'a {
-        self.0
-            .split_mut(|b| *b == b';')
-            // the `from_packet` method guarantees that the args are valid hex ascii, so this should
-            // method should never fail.
-            .map(|raw| decode_hex_buf(raw).unwrap_or(&mut []))
-            .map(|s| s as &[u8])
-            .filter(|s| !s.is_empty())
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

src/protocol/common/lists.rs

@@ -0,0 +1,26 @@
+use crate::protocol::common::hex::{decode_hex_buf, is_hex};
+
+/// A wrapper type around a list of hex encoded arguments separated by `;`.
+#[derive(Debug)]
+pub struct ArgListHex<'a>(&'a mut [u8]);
+
+impl<'a> ArgListHex<'a> {
+    pub fn from_packet(args: &'a mut [u8]) -> Option<Self> {
+        // validate that args have valid hex encoding (with ';' delimiters).
+        // this removes all the error handling from the lazy `Args` iterator.
+        if args.iter().any(|b| !(is_hex(*b) || *b == b';')) {
+            return None;
+        }
+        Some(Self(args))
+    }
+
+    pub fn into_iter(self) -> impl Iterator<Item = &'a [u8]> + 'a {
+        self.0
+            .split_mut(|b| *b == b';')
+            // the `from_packet` method guarantees that the args are valid hex ascii, so this should
+            // method should never fail.
+            .map(|raw| decode_hex_buf(raw).unwrap_or(&mut []))
+            .map(|s| s as &[u8])
+            .filter(|s| !s.is_empty())
+    }
+}

src/protocol/common/mod.rs

@@ -1,4 +1,5 @@
 pub mod hex;
+pub mod lists;
 pub mod thread_id;
 
 /// Lightweight wrapper around `&[u8]` which denotes that the contained data is

src/target/ext/catch_syscalls.rs

@@ -1,33 +1,33 @@
 //! Enable or disable catching syscalls from the inferior process.
 
 use crate::arch::Arch;
-use crate::target::Target;
+use crate::target::{Target, TargetResult};
 
 /// Target Extension - Enable and disable catching syscalls from the inferior
 /// process.
 ///
-/// Corresponds GDB's [`QCatchSyscalls`](https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#QCatchSyscalls) command.
+/// Corresponds to GDB's [`QCatchSyscalls`](https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#QCatchSyscalls) command.
 pub trait CatchSyscalls: Target {
     /// Enables catching syscalls from the inferior process.
     ///
-    /// If `filter` not `None`, then only the syscalls listed in the filter
-    /// should be reported to GDB.
+    /// If `filter` is `None`, then all syscalls should be reported to GDB. If a
+    /// filter is provided, only the syscalls listed in the filter should be
+    /// reported to GDB.
     ///
     /// Note: filters are not combined, subsequent calls this method should
     /// replace any existing syscall filtering.
     fn enable_catch_syscalls(
         &mut self,
         filter: Option<SyscallNumbers<<Self::Arch as Arch>::Usize>>,
-    ) -> Result<(), Self::Error>;
+    ) -> TargetResult<(), Self>;
 
     /// Disables catching syscalls from the inferior process.
-    fn disable_catch_syscalls(&mut self) -> Result<(), Self::Error>;
+    fn disable_catch_syscalls(&mut self) -> TargetResult<(), Self>;
 }
 
 define_ext!(CatchSyscallsOps, CatchSyscalls);
 
-/// Describes why a catch syscall event was triggered for the corresponding stop
-/// reason.
+/// Describes where the syscall catchpoint was triggered at.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum CatchSyscallPosition {
     /// Reached the entry location of the syscall.

src/target/mod.rs

@@ -317,12 +317,6 @@ pub trait Target {
         None
     }
 
-    /// Set/Remove syscall catchpoints.
-    #[inline(always)]
-    fn catch_syscalls(&mut self) -> Option<ext::catch_syscalls::CatchSyscallsOps<Self>> {
-        None
-    }
-
     /// Handle custom GDB `monitor` commands.
     #[inline(always)]
     fn monitor_cmd(&mut self) -> Option<ext::monitor_cmd::MonitorCmdOps<Self>> {
@@ -355,6 +349,12 @@ pub trait Target {
     fn memory_map(&mut self) -> Option<ext::memory_map::MemoryMapOps<Self>> {
         None
     }
+
+    /// Set/Remove syscall catchpoints.
+    #[inline(always)]
+    fn catch_syscalls(&mut self) -> Option<ext::catch_syscalls::CatchSyscallsOps<Self>> {
+        None
+    }
 }
 
 macro_rules! impl_dyn_target {