Avoid useless conversions

Use native C types to avoid platform-dependent conversions.
aya-rs · Dec 27, 2024 · fa9a6f2 · fa9a6f2
1 parent 41706d7
commit fa9a6f2
Show file tree

Hide file tree

Showing 11 changed files with 47 additions and 50 deletions.
diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs
@@ -1,11 +1,9 @@
 use std::{
     borrow::Cow,
     collections::{HashMap, HashSet},
+    ffi::c_int,
     fs, io,
-    os::{
-        fd::{AsFd as _, AsRawFd as _},
-        raw::c_int,
-    },
+    os::fd::{AsFd as _, AsRawFd as _},
     path::{Path, PathBuf},
     sync::{Arc, LazyLock},
 };

diff --git a/aya/src/maps/bloom_filter.rs b/aya/src/maps/bloom_filter.rs
@@ -78,7 +78,7 @@ impl<T: BorrowMut<MapData>, V: Pod> BloomFilter<T, V> {
 
 #[cfg(test)]
 mod tests {
-    use std::io;
+    use std::{ffi::c_long, io};
 
     use assert_matches::assert_matches;
     use libc::{EFAULT, ENOENT};
@@ -101,7 +101,7 @@ mod tests {
         test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_BLOOM_FILTER)
     }
 
-    fn sys_error(value: i32) -> SysResult<i64> {
+    fn sys_error(value: i32) -> SysResult<c_long> {
         Err((-1, io::Error::from_raw_os_error(value)))
     }
 

diff --git a/aya/src/maps/hash_map/hash_map.rs b/aya/src/maps/hash_map/hash_map.rs
@@ -103,7 +103,7 @@ impl<T: Borrow<MapData>, K: Pod, V: Pod> IterableMap<K, V> for HashMap<T, K, V>
 
 #[cfg(test)]
 mod tests {
-    use std::io;
+    use std::{ffi::c_long, io};
 
     use assert_matches::assert_matches;
     use libc::{EFAULT, ENOENT};
@@ -126,7 +126,7 @@ mod tests {
         test_utils::new_obj_map::<u32>(BPF_MAP_TYPE_HASH)
     }
 
-    fn sys_error(value: i32) -> SysResult<i64> {
+    fn sys_error(value: i32) -> SysResult<c_long> {
         Err((-1, io::Error::from_raw_os_error(value)))
     }
 
@@ -332,7 +332,7 @@ mod tests {
         assert_matches!(keys, Ok(ks) if ks.is_empty())
     }
 
-    fn get_next_key(attr: &bpf_attr) -> SysResult<i64> {
+    fn get_next_key(attr: &bpf_attr) -> SysResult<c_long> {
         match bpf_key(attr) {
             None => set_next_key(attr, 10),
             Some(10) => set_next_key(attr, 20),
@@ -344,7 +344,7 @@ mod tests {
         Ok(1)
     }
 
-    fn lookup_elem(attr: &bpf_attr) -> SysResult<i64> {
+    fn lookup_elem(attr: &bpf_attr) -> SysResult<c_long> {
         match bpf_key(attr) {
             Some(10) => set_ret(attr, 100),
             Some(20) => set_ret(attr, 200),

diff --git a/aya/src/maps/lpm_trie.rs b/aya/src/maps/lpm_trie.rs
@@ -195,7 +195,7 @@ impl<T: Borrow<MapData>, K: Pod, V: Pod> IterableMap<Key<K>, V> for LpmTrie<T, K
 
 #[cfg(test)]
 mod tests {
-    use std::{io, net::Ipv4Addr};
+    use std::{ffi::c_long, io, net::Ipv4Addr};
 
     use assert_matches::assert_matches;
     use libc::{EFAULT, ENOENT};
@@ -218,7 +218,7 @@ mod tests {
         test_utils::new_obj_map::<Key<u32>>(BPF_MAP_TYPE_LPM_TRIE)
     }
 
-    fn sys_error(value: i32) -> SysResult<i64> {
+    fn sys_error(value: i32) -> SysResult<c_long> {
         Err((-1, io::Error::from_raw_os_error(value)))
     }
 

diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs
@@ -49,7 +49,7 @@
 //! implement the [Pod] trait.
 use std::{
     borrow::Borrow,
-    ffi::CString,
+    ffi::{c_long, CString},
     fmt, io,
     marker::PhantomData,
     mem,
@@ -127,7 +127,7 @@ pub enum MapError {
         /// Map name
         name: String,
         /// Error code
-        code: i64,
+        code: c_long,
         #[source]
         /// Original io::Error
         io_error: io::Error,

diff --git a/aya/src/programs/uprobe.rs b/aya/src/programs/uprobe.rs
@@ -2,11 +2,11 @@
 use std::{
     borrow::Cow,
     error::Error,
-    ffi::{CStr, OsStr, OsString},
+    ffi::{c_char, CStr, OsStr, OsString},
     fs,
     io::{self, BufRead, Cursor, Read},
     mem,
-    os::{fd::AsFd as _, raw::c_char, unix::ffi::OsStrExt},
+    os::{fd::AsFd as _, unix::ffi::OsStrExt},
     path::{Path, PathBuf},
     sync::LazyLock,
 };

diff --git a/aya/src/sys/bpf.rs b/aya/src/sys/bpf.rs
@@ -1,6 +1,6 @@
 use std::{
     cmp,
-    ffi::{c_char, CStr, CString},
+    ffi::{c_char, c_long, CStr, CString},
     io, iter,
     mem::{self, MaybeUninit},
     os::fd::{AsFd as _, AsRawFd as _, BorrowedFd, FromRawFd as _, RawFd},
@@ -108,7 +108,7 @@ pub(crate) fn bpf_create_map(
     unsafe { fd_sys_bpf(bpf_cmd::BPF_MAP_CREATE, &mut attr) }
 }
 
-pub(crate) fn bpf_pin_object(fd: BorrowedFd<'_>, path: &CStr) -> SysResult<i64> {
+pub(crate) fn bpf_pin_object(fd: BorrowedFd<'_>, path: &CStr) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
     let u = unsafe { &mut attr.__bindgen_anon_4 };
     u.bpf_fd = fd.as_raw_fd() as u32;
@@ -289,7 +289,7 @@ pub(crate) fn bpf_map_update_elem<K: Pod, V: Pod>(
     key: Option<&K>,
     value: &V,
     flags: u64,
-) -> SysResult<i64> {
+) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     let u = unsafe { &mut attr.__bindgen_anon_2 };
@@ -307,7 +307,7 @@ pub(crate) fn bpf_map_push_elem<V: Pod>(
     fd: BorrowedFd<'_>,
     value: &V,
     flags: u64,
-) -> SysResult<i64> {
+) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     let u = unsafe { &mut attr.__bindgen_anon_2 };
@@ -323,7 +323,7 @@ pub(crate) fn bpf_map_update_elem_ptr<K, V>(
     key: *const K,
     value: *mut V,
     flags: u64,
-) -> SysResult<i64> {
+) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     let u = unsafe { &mut attr.__bindgen_anon_2 };
@@ -340,12 +340,12 @@ pub(crate) fn bpf_map_update_elem_per_cpu<K: Pod, V: Pod>(
     key: &K,
     values: &PerCpuValues<V>,
     flags: u64,
-) -> SysResult<i64> {
+) -> SysResult<c_long> {
     let mut mem = values.build_kernel_mem().map_err(|e| (-1, e))?;
     bpf_map_update_elem_ptr(fd, key, mem.as_mut_ptr(), flags)
 }
 
-pub(crate) fn bpf_map_delete_elem<K: Pod>(fd: BorrowedFd<'_>, key: &K) -> SysResult<i64> {
+pub(crate) fn bpf_map_delete_elem<K: Pod>(fd: BorrowedFd<'_>, key: &K) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     let u = unsafe { &mut attr.__bindgen_anon_2 };
@@ -377,7 +377,7 @@ pub(crate) fn bpf_map_get_next_key<K: Pod>(
 }
 
 // since kernel 5.2
-pub(crate) fn bpf_map_freeze(fd: BorrowedFd<'_>) -> SysResult<i64> {
+pub(crate) fn bpf_map_freeze(fd: BorrowedFd<'_>) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
     let u = unsafe { &mut attr.__bindgen_anon_2 };
     u.map_fd = fd.as_raw_fd() as u32;
@@ -453,7 +453,7 @@ pub(crate) fn bpf_link_update(
     new_prog_fd: BorrowedFd<'_>,
     old_prog_fd: Option<RawFd>,
     flags: u32,
-) -> SysResult<i64> {
+) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     attr.link_update.link_fd = link_fd.as_raw_fd() as u32;
@@ -528,7 +528,7 @@ pub(crate) fn bpf_prog_query(
     prog_ids: &mut [u32],
     prog_cnt: &mut u32,
     revision: &mut u64,
-) -> SysResult<i64> {
+) -> SysResult<c_long> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     match target {
@@ -1164,7 +1164,7 @@ fn bpf_prog_load(attr: &mut bpf_attr) -> SysResult<crate::MockableFd> {
     unsafe { fd_sys_bpf(bpf_cmd::BPF_PROG_LOAD, attr) }
 }
 
-fn sys_bpf(cmd: bpf_cmd, attr: &mut bpf_attr) -> SysResult<i64> {
+fn sys_bpf(cmd: bpf_cmd, attr: &mut bpf_attr) -> SysResult<c_long> {
     syscall(Syscall::Ebpf { cmd, attr })
 }
 

diff --git a/aya/src/sys/fake.rs b/aya/src/sys/fake.rs
@@ -1,8 +1,12 @@
-use std::{cell::RefCell, ffi::c_void, io, ptr};
+use std::{
+    cell::RefCell,
+    ffi::{c_long, c_void},
+    io, ptr,
+};
 
 use super::{SysResult, Syscall};
 
-type SyscallFn = unsafe fn(Syscall<'_>) -> SysResult<i64>;
+type SyscallFn = unsafe fn(Syscall<'_>) -> SysResult<c_long>;
 
 #[cfg(test)]
 thread_local! {
@@ -11,11 +15,11 @@ thread_local! {
 }
 
 #[cfg(test)]
-unsafe fn test_syscall(_call: Syscall<'_>) -> SysResult<i64> {
+unsafe fn test_syscall(_call: Syscall<'_>) -> SysResult<c_long> {
     Err((-1, io::Error::from_raw_os_error(libc::EINVAL)))
 }
 
 #[cfg(test)]
-pub(crate) fn override_syscall(call: unsafe fn(Syscall<'_>) -> SysResult<i64>) {
+pub(crate) fn override_syscall(call: unsafe fn(Syscall<'_>) -> SysResult<c_long>) {
     TEST_SYSCALL.with(|test_impl| *test_impl.borrow_mut() = call);
 }
diff --git a/aya/src/sys/mod.rs b/aya/src/sys/mod.rs
@@ -8,15 +8,15 @@ mod perf_event;
 mod fake;
 
 use std::{
-    ffi::{c_int, c_void},
+    ffi::{c_int, c_long, c_void},
     io, mem,
     os::fd::{AsRawFd as _, BorrowedFd, OwnedFd},
 };
 
 pub(crate) use bpf::*;
 #[cfg(test)]
 pub(crate) use fake::*;
-use libc::{pid_t, SYS_bpf, SYS_perf_event_open};
+use libc::{pid_t, SYS_bpf, SYS_ioctl, SYS_perf_event_open};
 #[doc(hidden)]
 pub use netlink::netlink_set_link_up;
 pub(crate) use netlink::*;
@@ -25,7 +25,7 @@ use thiserror::Error;
 
 use crate::generated::{bpf_attr, bpf_cmd, perf_event_attr};
 
-pub(crate) type SysResult<T> = Result<T, (i64, io::Error)>;
+pub(crate) type SysResult<T> = Result<T, (c_long, io::Error)>;
 
 pub(crate) enum Syscall<'a> {
     Ebpf {
@@ -89,7 +89,7 @@ impl std::fmt::Debug for Syscall<'_> {
     }
 }
 
-fn syscall(call: Syscall<'_>) -> SysResult<i64> {
+fn syscall(call: Syscall<'_>) -> SysResult<c_long> {
     #[cfg(test)]
     return TEST_SYSCALL.with(|test_impl| unsafe { test_impl.borrow()(call) });
 
@@ -108,22 +108,13 @@ fn syscall(call: Syscall<'_>) -> SysResult<i64> {
                     flags,
                 } => libc::syscall(SYS_perf_event_open, &attr, pid, cpu, group, flags),
                 Syscall::PerfEventIoctl { fd, request, arg } => {
-                    // The type of integer taken by `ioctl` is different in glibc (i64) and
-                    // musl (i32). musl builds would complain about useless conversion.
-                    #[allow(clippy::useless_conversion)]
-                    let request = request.try_into().unwrap();
-                    let ret = libc::ioctl(fd.as_raw_fd(), request, arg);
-                    // `libc::ioctl` returns i32 on x86_64 while `libc::syscall` returns i64.
-                    #[allow(clippy::useless_conversion)]
-                    ret.into()
+                    libc::syscall(SYS_ioctl, fd.as_raw_fd(), request, arg)
                 }
             }
         };
 
-        // `libc::syscall` returns i32 on armv7.
-        #[allow(clippy::useless_conversion)]
-        match ret.into() {
-            ret @ 0.. => Ok(ret),
+        match ret {
+            0.. => Ok(ret),
             ret => Err((ret, io::Error::last_os_error())),
         }
     }

diff --git a/aya/src/sys/perf_event.rs b/aya/src/sys/perf_event.rs
@@ -1,5 +1,5 @@
 use std::{
-    ffi::{c_int, CString, OsStr},
+    ffi::{c_int, c_long, CString, OsStr},
     io, mem,
     os::fd::{BorrowedFd, FromRawFd as _},
 };
@@ -104,7 +104,11 @@ pub(crate) fn perf_event_open_trace_point(
     perf_event_sys(attr, pid, cpu, PERF_FLAG_FD_CLOEXEC)
 }
 
-pub(crate) fn perf_event_ioctl(fd: BorrowedFd<'_>, request: c_int, arg: c_int) -> SysResult<i64> {
+pub(crate) fn perf_event_ioctl(
+    fd: BorrowedFd<'_>,
+    request: c_int,
+    arg: c_int,
+) -> SysResult<c_long> {
     let call = Syscall::PerfEventIoctl { fd, request, arg };
     #[cfg(not(test))]
     return syscall(call);

diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt
@@ -1336,7 +1336,7 @@ impl<T> core::convert::From<T> for aya::maps::Map
 pub fn aya::maps::Map::from(t: T) -> T
 pub enum aya::maps::MapError
 pub aya::maps::MapError::CreateError
-pub aya::maps::MapError::CreateError::code: i64
+pub aya::maps::MapError::CreateError::code: core::ffi::c_long
 pub aya::maps::MapError::CreateError::io_error: std::io::error::Error
 pub aya::maps::MapError::CreateError::name: alloc::string::String
 pub aya::maps::MapError::ElementNotFound