Browse Source

aya-log-common: Simplify

- Remove `TagLenValue`; this type has a single method, which is now a
  function.
- Remove generics from `TagLenValue::write` (now `write`). The tag is
  always `u8`, and the value is always a sequence of bytes.
- Replace slicing operations which can panic with calls to `get` which
  explicit check bounds.
Tamir Duberstein 1 year ago
parent
commit
fe047d79a3
2 changed files with 44 additions and 67 deletions
  1. 40 67
      aya-log-common/src/lib.rs
  2. 4 0
      xtask/public-api/aya-log-common.txt

+ 40 - 67
aya-log-common/src/lib.rs

@@ -1,6 +1,6 @@
 #![no_std]
 
-use core::{mem, num, ptr};
+use core::num;
 
 use num_enum::IntoPrimitive;
 
@@ -86,7 +86,7 @@ pub trait UpperMacFormatter {}
 impl UpperMacFormatter for [u8; 6] {}
 
 #[repr(u8)]
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, IntoPrimitive)]
 pub enum RecordField {
     Target = 1,
     Level,
@@ -99,7 +99,7 @@ pub enum RecordField {
 /// Types which are supported by aya-log and can be safely sent from eBPF
 /// programs to userspace.
 #[repr(u8)]
-#[derive(Copy, Clone, Debug)]
+#[derive(Copy, Clone, Debug, IntoPrimitive)]
 pub enum Argument {
     DisplayHint,
 
@@ -147,53 +147,24 @@ pub enum DisplayHint {
     UpperMac,
 }
 
-struct TagLenValue<T, V> {
-    pub tag: T,
-    pub value: V,
-}
-
-impl<T, V> TagLenValue<T, V>
-where
-    V: IntoIterator<Item = u8>,
-    <V as IntoIterator>::IntoIter: ExactSizeIterator,
-{
-    pub(crate) fn write(self, mut buf: &mut [u8]) -> Result<usize, ()> {
-        // Break the abstraction to please the verifier.
-        if buf.len() > LOG_BUF_CAPACITY {
-            buf = &mut buf[..LOG_BUF_CAPACITY];
-        }
-        let Self { tag, value } = self;
-        let value = value.into_iter();
-        let len = value.len();
-        let wire_len: LogValueLength = value
-            .len()
-            .try_into()
-            .map_err(|num::TryFromIntError { .. }| ())?;
-        let size = mem::size_of_val(&tag) + mem::size_of_val(&wire_len) + len;
-        if size > buf.len() {
-            return Err(());
-        }
-
-        let tag_size = mem::size_of_val(&tag);
-        unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, tag) };
-        buf = &mut buf[tag_size..];
-
-        unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, wire_len) };
-        buf = &mut buf[mem::size_of_val(&wire_len)..];
-
-        buf.iter_mut().zip(value).for_each(|(dst, src)| {
-            *dst = src;
-        });
-
-        Ok(size)
-    }
-}
-
-impl<T, V> TagLenValue<T, V> {
-    #[inline(always)]
-    pub(crate) fn new(tag: T, value: V) -> TagLenValue<T, V> {
-        TagLenValue { tag, value }
+pub(crate) fn write(tag: u8, value: &[u8], buf: &mut [u8]) -> Result<usize, ()> {
+    let wire_len: LogValueLength = value
+        .len()
+        .try_into()
+        .map_err(|num::TryFromIntError { .. }| ())?;
+    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(())?;
+            buf.copy_from_slice($value);
+            size += $value.len();
+        }};
     }
+    copy_from_slice!(&[tag]);
+    copy_from_slice!(&wire_len.to_ne_bytes());
+    copy_from_slice!(value);
+    Ok(size)
 }
 
 pub trait WriteToBuf {
@@ -205,7 +176,7 @@ macro_rules! impl_write_to_buf {
     ($type:ident, $arg_type:expr) => {
         impl WriteToBuf for $type {
             fn write(self, buf: &mut [u8]) -> Result<usize, ()> {
-                TagLenValue::new($arg_type, self.to_ne_bytes()).write(buf)
+                write($arg_type.into(), &self.to_ne_bytes(), buf)
             }
         }
     };
@@ -228,45 +199,45 @@ impl_write_to_buf!(f64, Argument::F64);
 
 impl WriteToBuf for [u8; 16] {
     fn write(self, buf: &mut [u8]) -> Result<usize, ()> {
-        TagLenValue::new(Argument::ArrU8Len16, self).write(buf)
+        write(Argument::ArrU8Len16.into(), &self, buf)
     }
 }
 
 impl WriteToBuf for [u16; 8] {
     fn write(self, buf: &mut [u8]) -> Result<usize, ()> {
         let bytes = unsafe { core::mem::transmute::<_, [u8; 16]>(self) };
-        TagLenValue::new(Argument::ArrU16Len8, bytes).write(buf)
+        write(Argument::ArrU16Len8.into(), &bytes, buf)
     }
 }
 
 impl WriteToBuf for [u8; 6] {
     fn write(self, buf: &mut [u8]) -> Result<usize, ()> {
-        TagLenValue::new(Argument::ArrU8Len6, self).write(buf)
+        write(Argument::ArrU8Len6.into(), &self, buf)
     }
 }
 
 impl WriteToBuf for &[u8] {
     fn write(self, buf: &mut [u8]) -> Result<usize, ()> {
-        TagLenValue::new(Argument::Bytes, self.iter().copied()).write(buf)
+        write(Argument::Bytes.into(), self, buf)
     }
 }
 
 impl WriteToBuf for &str {
     fn write(self, buf: &mut [u8]) -> Result<usize, ()> {
-        TagLenValue::new(Argument::Str, self.as_bytes().iter().copied()).write(buf)
+        write(Argument::Str.into(), self.as_bytes(), buf)
     }
 }
 
 impl WriteToBuf for DisplayHint {
     fn write(self, buf: &mut [u8]) -> Result<usize, ()> {
         let v: u8 = self.into();
-        TagLenValue::new(Argument::DisplayHint, v.to_ne_bytes()).write(buf)
+        write(Argument::DisplayHint.into(), &v.to_ne_bytes(), buf)
     }
 }
 
 #[allow(clippy::result_unit_err)]
 #[doc(hidden)]
-#[inline(always)]
+#[inline(always)] // This function takes too many arguments to not be inlined.
 pub fn write_record_header(
     buf: &mut [u8],
     target: &str,
@@ -278,16 +249,18 @@ pub fn write_record_header(
 ) -> Result<usize, ()> {
     let level: u8 = level.into();
     let mut size = 0;
-    size += TagLenValue::new(RecordField::Target, target.as_bytes().iter().copied())
-        .write(&mut buf[size..])?;
-    size += TagLenValue::new(RecordField::Level, level.to_ne_bytes()).write(&mut buf[size..])?;
-    size += TagLenValue::new(RecordField::Module, module.as_bytes().iter().copied())
-        .write(&mut buf[size..])?;
-    size += TagLenValue::new(RecordField::File, file.as_bytes().iter().copied())
-        .write(&mut buf[size..])?;
-    size += TagLenValue::new(RecordField::Line, line.to_ne_bytes()).write(&mut buf[size..])?;
-    size +=
-        TagLenValue::new(RecordField::NumArgs, num_args.to_ne_bytes()).write(&mut buf[size..])?;
+    macro_rules! write {
+        ($tag:expr, $value:expr) => {{
+            let buf = buf.get_mut(size..).ok_or(())?;
+            size += write($tag.into(), $value, buf)?;
+        }};
+    }
+    write!(RecordField::Target, target.as_bytes());
+    write!(RecordField::Level, &level.to_ne_bytes());
+    write!(RecordField::Module, module.as_bytes());
+    write!(RecordField::File, file.as_bytes());
+    write!(RecordField::Line, &line.to_ne_bytes());
+    write!(RecordField::NumArgs, &num_args.to_ne_bytes());
     Ok(size)
 }
 

+ 4 - 0
xtask/public-api/aya-log-common.txt

@@ -18,6 +18,8 @@ pub aya_log_common::Argument::U32
 pub aya_log_common::Argument::U64
 pub aya_log_common::Argument::U8
 pub aya_log_common::Argument::Usize
+impl core::convert::From<aya_log_common::Argument> for u8
+pub fn u8::from(enum_value: aya_log_common::Argument) -> Self
 impl core::clone::Clone for aya_log_common::Argument
 pub fn aya_log_common::Argument::clone(&self) -> aya_log_common::Argument
 impl core::fmt::Debug for aya_log_common::Argument
@@ -134,6 +136,8 @@ pub aya_log_common::RecordField::Line
 pub aya_log_common::RecordField::Module
 pub aya_log_common::RecordField::NumArgs
 pub aya_log_common::RecordField::Target = 1
+impl core::convert::From<aya_log_common::RecordField> for u8
+pub fn u8::from(enum_value: aya_log_common::RecordField) -> Self
 impl core::clone::Clone for aya_log_common::RecordField
 pub fn aya_log_common::RecordField::clone(&self) -> aya_log_common::RecordField
 impl core::fmt::Debug for aya_log_common::RecordField