re-impl the cmp expression

5 files changed, 95 insertions, 203 deletions

diff --git a/examples/add-rules.rs b/examples/add-rules.rs
index cd7423c..49e8b7b 100644
--- a/examples/add-rules.rs
+++ b/examples/add-rules.rs
@@ -82,7 +82,7 @@ fn main() -> Result<(), Error> {
     batch.add(&in_chain, MsgType::Add);
 
     let rule = Rule::new(&in_chain)?.with_expressions(
-        ExpressionList::builder().with_expression(Immediate::new_verdict(VerdictKind::Accept)),
+        ExpressionList::default().with_expression(Immediate::new_verdict(VerdictKind::Accept)),
     );
 
     batch.add(&rule, MsgType::Add);
diff --git a/src/expr/cmp.rs b/src/expr/cmp.rs
index d8f825f..d69f73c 100644
--- a/src/expr/cmp.rs
+++ b/src/expr/cmp.rs
@@ -1,162 +1,61 @@
-use super::{DeserializationError, Expression, Rule, ToSlice};
-use crate::sys::{self, libc};
-use std::{
-    borrow::Cow,
-    ffi::{c_void, CString},
-    os::raw::c_char,
+use rustables_macros::{nfnetlink_enum, nfnetlink_struct};
+
+use crate::sys::{
+    NFTA_CMP_DATA, NFTA_CMP_OP, NFTA_CMP_SREG, NFT_CMP_EQ, NFT_CMP_GT, NFT_CMP_GTE, NFT_CMP_LT,
+    NFT_CMP_LTE, NFT_CMP_NEQ,
 };
 
+use super::{Expression, ExpressionData, Register};
+
 /// Comparison operator.
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[nfnetlink_enum(u32, nested = true)]
 pub enum CmpOp {
     /// Equals.
-    Eq,
+    Eq = NFT_CMP_EQ,
     /// Not equal.
-    Neq,
+    Neq = NFT_CMP_NEQ,
     /// Less than.
-    Lt,
+    Lt = NFT_CMP_LT,
     /// Less than, or equal.
-    Lte,
+    Lte = NFT_CMP_LTE,
     /// Greater than.
-    Gt,
+    Gt = NFT_CMP_GT,
     /// Greater than, or equal.
-    Gte,
-}
-
-impl CmpOp {
-    /// Returns the corresponding `NFT_*` constant for this comparison operation.
-    pub fn to_raw(self) -> u32 {
-        use self::CmpOp::*;
-        match self {
-            Eq => libc::NFT_CMP_EQ as u32,
-            Neq => libc::NFT_CMP_NEQ as u32,
-            Lt => libc::NFT_CMP_LT as u32,
-            Lte => libc::NFT_CMP_LTE as u32,
-            Gt => libc::NFT_CMP_GT as u32,
-            Gte => libc::NFT_CMP_GTE as u32,
-        }
-    }
-
-    pub fn from_raw(val: u32) -> Result<Self, DeserializationError> {
-        use self::CmpOp::*;
-        match val as i32 {
-            libc::NFT_CMP_EQ => Ok(Eq),
-            libc::NFT_CMP_NEQ => Ok(Neq),
-            libc::NFT_CMP_LT => Ok(Lt),
-            libc::NFT_CMP_LTE => Ok(Lte),
-            libc::NFT_CMP_GT => Ok(Gt),
-            libc::NFT_CMP_GTE => Ok(Gte),
-            _ => Err(DeserializationError::InvalidValue),
-        }
-    }
+    Gte = NFT_CMP_GTE,
 }
 
 /// Comparator expression. Allows comparing the content of the netfilter register with any value.
-#[derive(Debug, PartialEq)]
-pub struct Cmp<T> {
+#[derive(Default, Debug, Clone, PartialEq, Eq)]
+#[nfnetlink_struct]
+pub struct Cmp {
+    #[field(NFTA_CMP_SREG)]
+    sreg: Register,
+    #[field(NFTA_CMP_OP)]
     op: CmpOp,
-    data: T,
+    #[field(NFTA_CMP_DATA)]
+    data: ExpressionData,
 }
 
-impl<T: ToSlice> Cmp<T> {
+impl Cmp {
     /// Returns a new comparison expression comparing the value loaded in the register with the
     /// data in `data` using the comparison operator `op`.
-    pub fn new(op: CmpOp, data: T) -> Self {
-        Cmp { op, data }
-    }
-}
-
-impl<T: ToSlice> Expression for Cmp<T> {
-    fn get_raw_name() -> *const c_char {
-        b"cmp\0" as *const _ as *const c_char
-    }
-
-    fn to_expr(&self, _rule: &Rule) -> *mut sys::nftnl_expr {
-        unsafe {
-            let expr = try_alloc!(sys::nftnl_expr_alloc(Self::get_raw_name()));
-
-            let data = self.data.to_slice();
-            trace!("Creating a cmp expr comparing with data {:?}", data);
-
-            sys::nftnl_expr_set_u32(
-                expr,
-                sys::NFTNL_EXPR_CMP_SREG as u16,
-                libc::NFT_REG_1 as u32,
-            );
-            sys::nftnl_expr_set_u32(expr, sys::NFTNL_EXPR_CMP_OP as u16, self.op.to_raw());
-            sys::nftnl_expr_set(
-                expr,
-                sys::NFTNL_EXPR_CMP_DATA as u16,
-                data.as_ptr() as *const c_void,
-                data.len() as u32,
-            );
-
-            expr
+    pub fn new(op: CmpOp, data: impl Into<Vec<u8>>) -> Self {
+        Cmp {
+            sreg: Some(Register::Reg1),
+            op: Some(op),
+            data: Some(ExpressionData::default().with_value(data)),
         }
     }
 }
 
-impl<const N: usize> Expression for Cmp<[u8; N]> {
-    fn get_raw_name() -> *const c_char {
-        Cmp::<u8>::get_raw_name()
-    }
-
-    /// The raw data contained inside `Cmp` expressions can only be deserialized to arrays of
-    /// bytes, to ensure that the memory layout of retrieved data cannot be violated. It is your
-    /// responsibility to provide the correct length of the byte data. If the data size is invalid,
-    /// you will get the error `DeserializationError::InvalidDataSize`.
-    ///
-    /// Example (warning, no error checking!):
-    /// ```rust
-    /// use std::ffi::CString;
-    /// use std::net::Ipv4Addr;
-    /// use std::rc::Rc;
-    ///
-    /// use rustables::{Chain, expr::{Cmp, CmpOp}, ProtoFamily, Rule, Table};
-    ///
-    /// let table = Rc::new(Table::new(&CString::new("mytable").unwrap(), ProtoFamily::Inet));
-    /// let chain = Rc::new(Chain::new(&CString::new("mychain").unwrap(), table));
-    /// let mut rule = Rule::new(chain);
-    /// rule.add_expr(&Cmp::new(CmpOp::Eq, 1337u16));
-    /// for expr in Rc::new(rule).get_exprs() {
-    ///     println!("{:?}", expr.decode_expr::<Cmp<[u8; 2]>>().unwrap());
-    /// }
-    /// ```
-    /// These limitations occur because casting bytes to any type of the same size
-    /// as the raw input would be *extremely* dangerous in terms of memory safety.
-    fn from_expr(expr: *const sys::nftnl_expr) -> Result<Self, DeserializationError> {
-        unsafe {
-            let ref_len = std::mem::size_of::<[u8; N]>() as u32;
-            let mut data_len = 0;
-            let data = sys::nftnl_expr_get(
-                expr,
-                sys::NFTNL_EXPR_CMP_DATA as u16,
-                &mut data_len as *mut u32,
-            );
-
-            if data.is_null() {
-                return Err(DeserializationError::NullPointer);
-            } else if data_len != ref_len {
-                return Err(DeserializationError::InvalidDataSize);
-            }
-
-            let data = *(data as *const [u8; N]);
-
-            let op = CmpOp::from_raw(sys::nftnl_expr_get_u32(expr, sys::NFTNL_EXPR_CMP_OP as u16))?;
-            Ok(Cmp { op, data })
-        }
-    }
-
-    // call to the other implementation to generate the expression
-    fn to_expr(&self, rule: &Rule) -> *mut sys::nftnl_expr {
-        Cmp {
-            data: &self.data as &[u8],
-            op: self.op,
-        }
-        .to_expr(rule)
+impl Expression for Cmp {
+    fn get_name() -> &'static str {
+        "cmp"
     }
 }
 
+/*
 /// Can be used to compare the value loaded by [`Meta::IifName`] and [`Meta::OifName`]. Please note
 /// that it is faster to check interface index than name.
 ///
@@ -182,13 +81,4 @@ impl ToSlice for InterfaceName {
         Cow::from(bytes)
     }
 }
-
-impl<'a> ToSlice for &'a InterfaceName {
-    fn to_slice(&self) -> Cow<'_, [u8]> {
-        let bytes = match *self {
-            InterfaceName::Exact(ref name) => name.as_bytes_with_nul(),
-            InterfaceName::StartingWith(ref name) => name.as_bytes(),
-        };
-        Cow::from(bytes)
-    }
-}
+*/
diff --git a/src/expr/mod.rs b/src/expr/mod.rs
index 8b3f5c2..0a7f54c 100644
--- a/src/expr/mod.rs
+++ b/src/expr/mod.rs
@@ -22,10 +22,8 @@ use thiserror::Error;
 mod bitwise;
 pub use self::bitwise::*;
 
-/*
 mod cmp;
 pub use self::cmp::*;
-*/
 
 mod counter;
 pub use self::counter::*;
@@ -225,7 +223,8 @@ create_expr_variant!(
     [Reject, Reject],
     [Counter, Counter],
     [Nat, Nat],
-    [Payload, Payload]
+    [Payload, Payload],
+    [Cmp, Cmp]
 );
 
 #[derive(Debug, Clone, PartialEq, Eq, Default)]
@@ -234,10 +233,6 @@ pub struct ExpressionList {
 }
 
 impl ExpressionList {
-    pub fn builder() -> Self {
-        Self { exprs: Vec::new() }
-    }
-
     /// Useful to add raw expressions because RawExpression cannot infer alone its type
     pub fn add_raw_expression(&mut self, e: RawExpression) {
         self.exprs.push(e);
diff --git a/src/parser.rs b/src/parser.rs
index 17905c4..b5e9f18 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -82,6 +82,9 @@ pub enum DecodeError {
     #[error("Invalid type for a payload expression")]
     UnknownPayloadType(u32),
 
+    #[error("Invalid type for a compare expression")]
+    UnknownCmpOp(u32),
+
     #[error("Unsupported value for a link layer header field")]
     UnknownLinkLayerHeaderField(u32, u32),
 
diff --git a/tests/expr.rs b/tests/expr.rs
index 2d0e12a..3b71c12 100644
--- a/tests/expr.rs
+++ b/tests/expr.rs
@@ -1,17 +1,18 @@
 use rustables::{
     expr::{
-        Bitwise, ExpressionList, HeaderField, HighLevelPayload, IcmpCode, Immediate, Log, Meta,
-        MetaType, Nat, NatType, Register, Reject, RejectType, TCPHeaderField, TransportHeaderField,
-        VerdictKind,
+        Bitwise, Cmp, CmpOp, ExpressionList, HeaderField, HighLevelPayload, IcmpCode, Immediate,
+        Log, Meta, MetaType, Nat, NatType, Register, Reject, RejectType, TCPHeaderField,
+        TransportHeaderField, VerdictKind,
     },
     sys::{
         NFTA_BITWISE_DREG, NFTA_BITWISE_LEN, NFTA_BITWISE_MASK, NFTA_BITWISE_SREG,
-        NFTA_BITWISE_XOR, NFTA_DATA_VALUE, NFTA_DATA_VERDICT, NFTA_EXPR_DATA, NFTA_EXPR_NAME,
-        NFTA_IMMEDIATE_DATA, NFTA_IMMEDIATE_DREG, NFTA_LIST_ELEM, NFTA_LOG_GROUP, NFTA_LOG_PREFIX,
-        NFTA_META_DREG, NFTA_META_KEY, NFTA_NAT_FAMILY, NFTA_NAT_REG_ADDR_MIN, NFTA_NAT_TYPE,
-        NFTA_PAYLOAD_BASE, NFTA_PAYLOAD_DREG, NFTA_PAYLOAD_LEN, NFTA_PAYLOAD_OFFSET,
-        NFTA_REJECT_ICMP_CODE, NFTA_REJECT_TYPE, NFTA_RULE_CHAIN, NFTA_RULE_EXPRESSIONS,
-        NFTA_RULE_TABLE, NFTA_VERDICT_CODE, NFT_META_PROTOCOL, NFT_NAT_SNAT,
+        NFTA_BITWISE_XOR, NFTA_CMP_DATA, NFTA_CMP_OP, NFTA_CMP_SREG, NFTA_DATA_VALUE,
+        NFTA_DATA_VERDICT, NFTA_EXPR_DATA, NFTA_EXPR_NAME, NFTA_IMMEDIATE_DATA,
+        NFTA_IMMEDIATE_DREG, NFTA_LIST_ELEM, NFTA_LOG_GROUP, NFTA_LOG_PREFIX, NFTA_META_DREG,
+        NFTA_META_KEY, NFTA_NAT_FAMILY, NFTA_NAT_REG_ADDR_MIN, NFTA_NAT_TYPE, NFTA_PAYLOAD_BASE,
+        NFTA_PAYLOAD_DREG, NFTA_PAYLOAD_LEN, NFTA_PAYLOAD_OFFSET, NFTA_REJECT_ICMP_CODE,
+        NFTA_REJECT_TYPE, NFTA_RULE_CHAIN, NFTA_RULE_EXPRESSIONS, NFTA_RULE_TABLE,
+        NFTA_VERDICT_CODE, NFT_CMP_EQ, NFT_META_PROTOCOL, NFT_NAT_SNAT,
         NFT_PAYLOAD_TRANSPORT_HEADER, NFT_REG_1, NFT_REG_VERDICT, NFT_REJECT_ICMPX_UNREACH,
     },
     ProtocolFamily,
@@ -38,7 +39,7 @@ fn bitwise_expr_is_valid() {
     let netmask = Ipv4Addr::new(255, 255, 255, 0);
     let bitwise = Bitwise::new(netmask.octets(), [0, 0, 0, 0]).unwrap();
     let mut rule =
-        get_test_rule().with_expressions(ExpressionList::builder().with_expression(bitwise));
+        get_test_rule().with_expressions(ExpressionList::default().with_expression(bitwise));
 
     let mut buf = Vec::new();
     let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg(&mut buf, &mut rule);
@@ -90,44 +91,47 @@ fn bitwise_expr_is_valid() {
         .to_raw()
     );
 }
-//
-//#[test]
-//fn cmp_expr_is_valid() {
-//    let cmp = Cmp::new(CmpOp::Eq, 0);
-//    let mut rule = get_test_rule();
-//    let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg_from_expr(&mut rule, &cmp);
-//    assert_eq!(nlmsghdr.nlmsg_len, 100);
-//
-//    assert_eq!(
-//        raw_expr,
-//        NetlinkExpr::List(vec![
-//            NetlinkExpr::Final(NFTA_RULE_TABLE, TABLE_NAME.to_vec()),
-//            NetlinkExpr::Final(NFTA_RULE_CHAIN, CHAIN_NAME.to_vec()),
-//            NetlinkExpr::Nested(
-//                NFTA_RULE_EXPRESSIONS,
-//                vec![NetlinkExpr::Nested(
-//                    NFTA_LIST_ELEM,
-//                    vec![
-//                        NetlinkExpr::Final(NFTA_EXPR_NAME, b"cmp\0".to_vec()),
-//                        NetlinkExpr::Nested(
-//                            NFTA_EXPR_DATA,
-//                            vec![
-//                                NetlinkExpr::Final(NFTA_CMP_SREG, NFT_REG_1.to_be_bytes().to_vec()),
-//                                NetlinkExpr::Final(NFTA_CMP_OP, NFT_CMP_EQ.to_be_bytes().to_vec()),
-//                                NetlinkExpr::Nested(
-//                                    NFTA_CMP_DATA,
-//                                    vec![NetlinkExpr::Final(1u16, 0u32.to_be_bytes().to_vec())]
-//                                )
-//                            ]
-//                        )
-//                    ]
-//                )]
-//            )
-//        ])
-//        .to_raw()
-//    );
-//}
-//
+
+#[test]
+fn cmp_expr_is_valid() {
+    let val = vec![1u8, 2, 3, 4];
+    let cmp = Cmp::new(CmpOp::Eq, val.clone());
+    let mut rule = get_test_rule().with_expressions(vec![cmp]);
+
+    let mut buf = Vec::new();
+    let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg(&mut buf, &mut rule);
+    assert_eq!(nlmsghdr.nlmsg_len, 100);
+
+    assert_eq!(
+        raw_expr,
+        NetlinkExpr::List(vec![
+            NetlinkExpr::Final(NFTA_RULE_TABLE, TABLE_NAME.as_bytes().to_vec()),
+            NetlinkExpr::Final(NFTA_RULE_CHAIN, CHAIN_NAME.as_bytes().to_vec()),
+            NetlinkExpr::Nested(
+                NFTA_RULE_EXPRESSIONS,
+                vec![NetlinkExpr::Nested(
+                    NFTA_LIST_ELEM,
+                    vec![
+                        NetlinkExpr::Final(NFTA_EXPR_NAME, b"cmp".to_vec()),
+                        NetlinkExpr::Nested(
+                            NFTA_EXPR_DATA,
+                            vec![
+                                NetlinkExpr::Final(NFTA_CMP_SREG, NFT_REG_1.to_be_bytes().to_vec()),
+                                NetlinkExpr::Final(NFTA_CMP_OP, NFT_CMP_EQ.to_be_bytes().to_vec()),
+                                NetlinkExpr::Nested(
+                                    NFTA_CMP_DATA,
+                                    vec![NetlinkExpr::Final(NFTA_DATA_VALUE, val)]
+                                )
+                            ]
+                        )
+                    ]
+                )]
+            )
+        ])
+        .to_raw()
+    );
+}
+
 //#[test]
 //fn counter_expr_is_valid() {
 //    let nb_bytes = 123456u64;
@@ -212,7 +216,7 @@ fn bitwise_expr_is_valid() {
 fn immediate_expr_is_valid() {
     let immediate = Immediate::new_data(vec![42u8], Register::Reg1);
     let mut rule =
-        get_test_rule().with_expressions(ExpressionList::builder().with_expression(immediate));
+        get_test_rule().with_expressions(ExpressionList::default().with_expression(immediate));
 
     let mut buf = Vec::new();
     let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg(&mut buf, &mut rule);
@@ -253,7 +257,7 @@ fn immediate_expr_is_valid() {
 #[test]
 fn log_expr_is_valid() {
     let log = Log::new(Some(1337), Some("mockprefix")).expect("Could not build a log expression");
-    let mut rule = get_test_rule().with_expressions(ExpressionList::builder().with_expression(log));
+    let mut rule = get_test_rule().with_expressions(ExpressionList::default().with_expression(log));
 
     let mut buf = Vec::new();
     let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg(&mut buf, &mut rule);
@@ -543,8 +547,8 @@ fn reject_expr_is_valid() {
 #[test]
 fn verdict_expr_is_valid() {
     let verdict = Immediate::new_verdict(VerdictKind::Drop);
-    let mut rule = get_test_rule();
-    rule.set_expressions(ExpressionList::builder().with_expression(verdict));
+    let mut rule =
+        get_test_rule().with_expressions(ExpressionList::default().with_expression(verdict));
 
     let mut buf = Vec::new();
     let (nlmsghdr, _nfgenmsg, raw_expr) = get_test_nlmsg(&mut buf, &mut rule);