From ca3f70b16a705bf26d2ccc7ce754de403be36223 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 9 Aug 2023 16:06:49 -0400 Subject: [PATCH] aya-log: s/Result/Option/ `Option` is guaranteed to have the same size as `usize`, which is not guarnateed for `Result`. This is a minor optimization, but also results in simpler code. --- aya-log-common/src/lib.rs | 45 +++++++------- aya-log-ebpf-macros/src/expand.rs | 14 +++-- aya-log/src/lib.rs | 93 ++++++++++++++++++----------- xtask/public-api/aya-log-common.txt | 40 ++++++------- 4 files changed, 107 insertions(+), 85 deletions(-) diff --git a/aya-log-common/src/lib.rs b/aya-log-common/src/lib.rs index ba20876f..f6cb3517 100644 --- a/aya-log-common/src/lib.rs +++ b/aya-log-common/src/lib.rs @@ -1,6 +1,6 @@ #![no_std] -use core::num; +use core::num::{NonZeroUsize, TryFromIntError}; use num_enum::IntoPrimitive; @@ -151,16 +151,16 @@ pub enum DisplayHint { // // llvm: :0:0: in function _ZN14aya_log_common5write17hc9ed05433e23a663E { i64, i64 } (i8, ptr, i64, ptr, i64): only integer returns supported #[inline(always)] -pub(crate) fn write(tag: u8, value: &[u8], buf: &mut [u8]) -> Result { - let wire_len: LogValueLength = value - .len() - .try_into() - .map_err(|num::TryFromIntError { .. }| ())?; +pub(crate) fn write(tag: u8, value: &[u8], buf: &mut [u8]) -> Option { + let wire_len: LogValueLength = match value.len().try_into() { + Ok(wire_len) => Some(wire_len), + Err(TryFromIntError { .. }) => None, + }?; let mut size = 0; macro_rules! copy_from_slice { ($value:expr) => {{ - let buf = buf.get_mut(size..).ok_or(())?; - let buf = buf.get_mut(..$value.len()).ok_or(())?; + let buf = buf.get_mut(size..)?; + let buf = buf.get_mut(..$value.len())?; buf.copy_from_slice($value); size += $value.len(); }}; @@ -168,12 +168,11 @@ pub(crate) fn write(tag: u8, value: &[u8], buf: &mut [u8]) -> Result copy_from_slice!(&[tag]); copy_from_slice!(&wire_len.to_ne_bytes()); copy_from_slice!(value); - Ok(size) + NonZeroUsize::new(size) } pub trait WriteToBuf { - #[allow(clippy::result_unit_err)] - fn write(self, buf: &mut [u8]) -> Result; + fn write(self, buf: &mut [u8]) -> Option; } macro_rules! impl_write_to_buf { @@ -182,7 +181,7 @@ macro_rules! impl_write_to_buf { // This need not be inlined because the return value is Result where N is // mem::size_of<$type>, which is a compile-time constant. #[inline(never)] - fn write(self, buf: &mut [u8]) -> Result { + fn write(self, buf: &mut [u8]) -> Option { write($arg_type.into(), &self.to_ne_bytes(), buf) } } @@ -208,7 +207,7 @@ impl WriteToBuf for [u8; 16] { // This need not be inlined because the return value is Result where N is 16, which is a // compile-time constant. #[inline(never)] - fn write(self, buf: &mut [u8]) -> Result { + fn write(self, buf: &mut [u8]) -> Option { write(Argument::ArrU8Len16.into(), &self, buf) } } @@ -217,7 +216,7 @@ impl WriteToBuf for [u16; 8] { // This need not be inlined because the return value is Result where N is 16, which is a // compile-time constant. #[inline(never)] - fn write(self, buf: &mut [u8]) -> Result { + fn write(self, buf: &mut [u8]) -> Option { let bytes = unsafe { core::mem::transmute::<_, [u8; 16]>(self) }; write(Argument::ArrU16Len8.into(), &bytes, buf) } @@ -227,7 +226,7 @@ impl WriteToBuf for [u8; 6] { // This need not be inlined because the return value is Result where N is 6, which is a // compile-time constant. #[inline(never)] - fn write(self, buf: &mut [u8]) -> Result { + fn write(self, buf: &mut [u8]) -> Option { write(Argument::ArrU8Len6.into(), &self, buf) } } @@ -237,7 +236,7 @@ impl WriteToBuf for &[u8] { // // llvm: :0:0: in function _ZN63_$LT$$RF$$u5b$u8$u5d$$u20$as$u20$aya_log_common..WriteToBuf$GT$5write17h08f30a45f7b9f09dE { i64, i64 } (ptr, i64, ptr, i64): only integer returns supported #[inline(always)] - fn write(self, buf: &mut [u8]) -> Result { + fn write(self, buf: &mut [u8]) -> Option { write(Argument::Bytes.into(), self, buf) } } @@ -247,7 +246,7 @@ impl WriteToBuf for &str { // // llvm: :0:0: in function _ZN54_$LT$$RF$str$u20$as$u20$aya_log_common..WriteToBuf$GT$5write17h7e2d1ccaa758e2b5E { i64, i64 } (ptr, i64, ptr, i64): only integer returns supported #[inline(always)] - fn write(self, buf: &mut [u8]) -> Result { + fn write(self, buf: &mut [u8]) -> Option { write(Argument::Str.into(), self.as_bytes(), buf) } } @@ -256,13 +255,12 @@ impl WriteToBuf for DisplayHint { // This need not be inlined because the return value is Result where N is 1, which is a // compile-time constant. #[inline(never)] - fn write(self, buf: &mut [u8]) -> Result { + fn write(self, buf: &mut [u8]) -> Option { let v: u8 = self.into(); write(Argument::DisplayHint.into(), &v.to_ne_bytes(), buf) } } -#[allow(clippy::result_unit_err)] #[doc(hidden)] #[inline(always)] // This function takes too many arguments to not be inlined. pub fn write_record_header( @@ -273,13 +271,14 @@ pub fn write_record_header( file: &str, line: u32, num_args: usize, -) -> Result { +) -> Option { let level: u8 = level.into(); let mut size = 0; macro_rules! write { ($tag:expr, $value:expr) => {{ - let buf = buf.get_mut(size..).ok_or(())?; - size += write($tag.into(), $value, buf)?; + let buf = buf.get_mut(size..)?; + let len = write($tag.into(), $value, buf)?; + size += len.get(); }}; } write!(RecordField::Target, target.as_bytes()); @@ -288,7 +287,7 @@ pub fn write_record_header( write!(RecordField::File, file.as_bytes()); write!(RecordField::Line, &line.to_ne_bytes()); write!(RecordField::NumArgs, &num_args.to_ne_bytes()); - Ok(size) + NonZeroUsize::new(size) } #[cfg(test)] diff --git a/aya-log-ebpf-macros/src/expand.rs b/aya-log-ebpf-macros/src/expand.rs index 78c92b47..ffbe3dbe 100644 --- a/aya-log-ebpf-macros/src/expand.rs +++ b/aya-log-ebpf-macros/src/expand.rs @@ -147,8 +147,8 @@ pub(crate) fn log(args: LogArgs, level: Option) -> Result {}, Some(::aya_log_ebpf::LogBuf { buf }) => { - let _: Result<(), ()> = (|| { - let mut len = ::aya_log_ebpf::write_record_header( + let _: Option<()> = (|| { + let size = ::aya_log_ebpf::write_record_header( buf, #target, #lvl, @@ -157,13 +157,15 @@ pub(crate) fn log(args: LogArgs, level: Option) -> Result Result<(usize, Vec), ()> { + fn new_log(args: usize) -> Option<(usize, Vec)> { let mut buf = vec![0; 8192]; let len = write_record_header( &mut buf, @@ -595,7 +595,7 @@ mod test { 123, args, )?; - Ok((len, buf)) + Some((len.get(), buf)) } #[test] @@ -603,7 +603,7 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(1).unwrap(); - len += "test".write(&mut input[len..]).unwrap(); + len += "test".write(&mut input[len..]).unwrap().get(); _ = len; @@ -621,8 +621,8 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(2).unwrap(); - len += "hello ".write(&mut input[len..]).unwrap(); - len += "test".write(&mut input[len..]).unwrap(); + len += "hello ".write(&mut input[len..]).unwrap().get(); + len += "test".write(&mut input[len..]).unwrap().get(); _ = len; @@ -640,8 +640,11 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(2).unwrap(); - len += DisplayHint::LowerHex.write(&mut input[len..]).unwrap(); - len += [0xde, 0xad].write(&mut input[len..]).unwrap(); + len += DisplayHint::LowerHex + .write(&mut input[len..]) + .unwrap() + .get(); + len += [0xde, 0xad].write(&mut input[len..]).unwrap().get(); _ = len; @@ -659,13 +662,19 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(5).unwrap(); - len += DisplayHint::LowerHex.write(&mut input[len..]).unwrap(); - len += [0xde, 0xad].write(&mut input[len..]).unwrap(); + len += DisplayHint::LowerHex + .write(&mut input[len..]) + .unwrap() + .get(); + len += [0xde, 0xad].write(&mut input[len..]).unwrap().get(); - len += " ".write(&mut input[len..]).unwrap(); + len += " ".write(&mut input[len..]).unwrap().get(); - len += DisplayHint::UpperHex.write(&mut input[len..]).unwrap(); - len += [0xbe, 0xef].write(&mut input[len..]).unwrap(); + len += DisplayHint::UpperHex + .write(&mut input[len..]) + .unwrap() + .get(); + len += [0xbe, 0xef].write(&mut input[len..]).unwrap().get(); _ = len; @@ -683,9 +692,9 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "default hint: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::Default.write(&mut input[len..]).unwrap(); - len += 14.write(&mut input[len..]).unwrap(); + len += "default hint: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::Default.write(&mut input[len..]).unwrap().get(); + len += 14.write(&mut input[len..]).unwrap().get(); _ = len; @@ -703,9 +712,12 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "lower hex: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::LowerHex.write(&mut input[len..]).unwrap(); - len += 200.write(&mut input[len..]).unwrap(); + len += "lower hex: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::LowerHex + .write(&mut input[len..]) + .unwrap() + .get(); + len += 200.write(&mut input[len..]).unwrap().get(); _ = len; @@ -723,9 +735,12 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "upper hex: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::UpperHex.write(&mut input[len..]).unwrap(); - len += 200.write(&mut input[len..]).unwrap(); + len += "upper hex: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::UpperHex + .write(&mut input[len..]) + .unwrap() + .get(); + len += 200.write(&mut input[len..]).unwrap().get(); _ = len; @@ -743,10 +758,10 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "ipv4: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::Ip.write(&mut input[len..]).unwrap(); + len += "ipv4: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::Ip.write(&mut input[len..]).unwrap().get(); // 10.0.0.1 as u32 - len += 167772161u32.write(&mut input[len..]).unwrap(); + len += 167772161u32.write(&mut input[len..]).unwrap().get(); _ = len; @@ -764,14 +779,14 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "ipv6: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::Ip.write(&mut input[len..]).unwrap(); + len += "ipv6: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::Ip.write(&mut input[len..]).unwrap().get(); // 2001:db8::1:1 as byte array let ipv6_arr: [u8; 16] = [ 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, ]; - len += ipv6_arr.write(&mut input[len..]).unwrap(); + len += ipv6_arr.write(&mut input[len..]).unwrap().get(); _ = len; @@ -789,13 +804,13 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "ipv6: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::Ip.write(&mut input[len..]).unwrap(); + len += "ipv6: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::Ip.write(&mut input[len..]).unwrap().get(); // 2001:db8::1:1 as u16 array let ipv6_arr: [u16; 8] = [ 0x2001, 0x0db8, 0x0000, 0x0000, 0x0000, 0x0000, 0x0001, 0x0001, ]; - len += ipv6_arr.write(&mut input[len..]).unwrap(); + len += ipv6_arr.write(&mut input[len..]).unwrap().get(); _ = len; @@ -813,11 +828,14 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "mac: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::LowerMac.write(&mut input[len..]).unwrap(); + len += "mac: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::LowerMac + .write(&mut input[len..]) + .unwrap() + .get(); // 00:00:5e:00:53:af as byte array let mac_arr: [u8; 6] = [0x00, 0x00, 0x5e, 0x00, 0x53, 0xaf]; - len += mac_arr.write(&mut input[len..]).unwrap(); + len += mac_arr.write(&mut input[len..]).unwrap().get(); _ = len; @@ -835,11 +853,14 @@ mod test { testing_logger::setup(); let (mut len, mut input) = new_log(3).unwrap(); - len += "mac: ".write(&mut input[len..]).unwrap(); - len += DisplayHint::UpperMac.write(&mut input[len..]).unwrap(); + len += "mac: ".write(&mut input[len..]).unwrap().get(); + len += DisplayHint::UpperMac + .write(&mut input[len..]) + .unwrap() + .get(); // 00:00:5E:00:53:AF as byte array let mac_arr: [u8; 6] = [0x00, 0x00, 0x5e, 0x00, 0x53, 0xaf]; - len += mac_arr.write(&mut input[len..]).unwrap(); + len += mac_arr.write(&mut input[len..]).unwrap().get(); _ = len; diff --git a/xtask/public-api/aya-log-common.txt b/xtask/public-api/aya-log-common.txt index 1b3e48c1..fd921d9b 100644 --- a/xtask/public-api/aya-log-common.txt +++ b/xtask/public-api/aya-log-common.txt @@ -54,7 +54,7 @@ pub aya_log_common::DisplayHint::LowerMac pub aya_log_common::DisplayHint::UpperHex pub aya_log_common::DisplayHint::UpperMac impl aya_log_common::WriteToBuf for aya_log_common::DisplayHint -pub fn aya_log_common::DisplayHint::write(self, buf: &mut [u8]) -> core::result::Result +pub fn aya_log_common::DisplayHint::write(self, buf: &mut [u8]) -> core::option::Option impl core::convert::From for u8 pub fn u8::from(enum_value: aya_log_common::DisplayHint) -> Self impl core::clone::Clone for aya_log_common::DisplayHint @@ -216,41 +216,41 @@ impl aya_log_common::UpperHexFormatter for usize pub trait aya_log_common::UpperMacFormatter impl aya_log_common::UpperMacFormatter for [u8; 6] pub trait aya_log_common::WriteToBuf -pub fn aya_log_common::WriteToBuf::write(self, buf: &mut [u8]) -> core::result::Result +pub fn aya_log_common::WriteToBuf::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for &[u8] -pub fn &[u8]::write(self, buf: &mut [u8]) -> core::result::Result +pub fn &[u8]::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for &str -pub fn &str::write(self, buf: &mut [u8]) -> core::result::Result +pub fn &str::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for [u16; 8] -pub fn [u16; 8]::write(self, buf: &mut [u8]) -> core::result::Result +pub fn [u16; 8]::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for [u8; 16] -pub fn [u8; 16]::write(self, buf: &mut [u8]) -> core::result::Result +pub fn [u8; 16]::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for [u8; 6] -pub fn [u8; 6]::write(self, buf: &mut [u8]) -> core::result::Result +pub fn [u8; 6]::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for aya_log_common::DisplayHint -pub fn aya_log_common::DisplayHint::write(self, buf: &mut [u8]) -> core::result::Result +pub fn aya_log_common::DisplayHint::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for f32 -pub fn f32::write(self, buf: &mut [u8]) -> core::result::Result +pub fn f32::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for f64 -pub fn f64::write(self, buf: &mut [u8]) -> core::result::Result +pub fn f64::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for i16 -pub fn i16::write(self, buf: &mut [u8]) -> core::result::Result +pub fn i16::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for i32 -pub fn i32::write(self, buf: &mut [u8]) -> core::result::Result +pub fn i32::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for i64 -pub fn i64::write(self, buf: &mut [u8]) -> core::result::Result +pub fn i64::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for i8 -pub fn i8::write(self, buf: &mut [u8]) -> core::result::Result +pub fn i8::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for isize -pub fn isize::write(self, buf: &mut [u8]) -> core::result::Result +pub fn isize::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for u16 -pub fn u16::write(self, buf: &mut [u8]) -> core::result::Result +pub fn u16::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for u32 -pub fn u32::write(self, buf: &mut [u8]) -> core::result::Result +pub fn u32::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for u64 -pub fn u64::write(self, buf: &mut [u8]) -> core::result::Result +pub fn u64::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for u8 -pub fn u8::write(self, buf: &mut [u8]) -> core::result::Result +pub fn u8::write(self, buf: &mut [u8]) -> core::option::Option impl aya_log_common::WriteToBuf for usize -pub fn usize::write(self, buf: &mut [u8]) -> core::result::Result +pub fn usize::write(self, buf: &mut [u8]) -> core::option::Option pub type aya_log_common::LogValueLength = u16