Revert "Merge NetlinkPayload::{Ack,Error}"

This reverts commit 52732b3.

Signed-off-by: Gris Ge <[email protected]>

Author: cathay4t

src/error.rs

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-use std::{fmt, io, mem::size_of, num::NonZeroI32};
+use std::{fmt, io, mem::size_of};
 
 use byteorder::{ByteOrder, NativeEndian};
 use netlink_packet_utils::DecodeError;
@@ -46,14 +46,10 @@ impl<T: AsRef<[u8]>> ErrorBuffer<T> {
         }
     }
 
-    /// Return the error code.
-    ///
-    /// Returns `None` when there is no error to report (the message is an ACK),
-    /// or a `Some(e)` if there is a non-zero error code `e` to report (the
-    /// message is a NACK).
-    pub fn code(&self) -> Option<NonZeroI32> {
+    /// Return the error code
+    pub fn code(&self) -> i32 {
         let data = self.buffer.as_ref();
-        NonZeroI32::new(NativeEndian::read_i32(&data[CODE]))
+        NativeEndian::read_i32(&data[CODE])
     }
 }
 
@@ -81,36 +77,22 @@ impl<T: AsRef<[u8]> + AsMut<[u8]>> ErrorBuffer<T> {
     }
 }
 
-/// An `NLMSG_ERROR` message.
-///
-/// Per [RFC 3549 section 2.3.2.2], this message carries the return code for a
-/// request which will indicate either success (an ACK) or failure (a NACK).
-///
-/// [RFC 3549 section 2.3.2.2]: https://datatracker.ietf.org/doc/html/rfc3549#section-2.3.2.2
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 pub struct ErrorMessage {
-    /// The error code.
-    ///
-    /// Holds `None` when there is no error to report (the message is an ACK),
-    /// or a `Some(e)` if there is a non-zero error code `e` to report (the
-    /// message is a NACK).
-    ///
-    /// See [Netlink message types] for details.
-    ///
-    /// [Netlink message types]: https://kernel.org/doc/html/next/userspace-api/netlink/intro.html#netlink-message-types
-    pub code: Option<NonZeroI32>,
-    /// The original request's header.
+    pub code: i32,
     pub header: Vec<u8>,
 }
 
+pub type AckMessage = ErrorMessage;
+
 impl Emitable for ErrorMessage {
     fn buffer_len(&self) -> usize {
         size_of::<i32>() + self.header.len()
     }
     fn emit(&self, buffer: &mut [u8]) {
         let mut buffer = ErrorBuffer::new(buffer);
-        buffer.set_code(self.raw_code());
+        buffer.set_code(self.code);
         buffer.payload_mut().copy_from_slice(&self.header)
     }
 }
@@ -137,18 +119,13 @@ impl<'buffer, T: AsRef<[u8]> + 'buffer> Parseable<ErrorBuffer<&'buffer T>>
 }
 
 impl ErrorMessage {
-    /// Returns the raw error code.
-    pub fn raw_code(&self) -> i32 {
-        self.code.map_or(0, NonZeroI32::get)
-    }
-
     /// According to [`netlink(7)`](https://linux.die.net/man/7/netlink)
     /// the `NLMSG_ERROR` return Negative errno or 0 for acknowledgements.
     ///
     /// convert into [`std::io::Error`](https://doc.rust-lang.org/std/io/struct.Error.html)
     /// using the absolute value from errno code
     pub fn to_io(&self) -> io::Error {
-        io::Error::from_raw_os_error(self.raw_code().abs())
+        io::Error::from_raw_os_error(self.code.abs())
     }
 }
 
@@ -172,7 +149,7 @@ mod tests {
     fn into_io_error() {
         let io_err = io::Error::from_raw_os_error(95);
         let err_msg = ErrorMessage {
-            code: NonZeroI32::new(-95),
+            code: -95,
             header: vec![],
         };
 
@@ -181,40 +158,4 @@ mod tests {
         assert_eq!(err_msg.to_string(), io_err.to_string());
         assert_eq!(to_io.raw_os_error(), io_err.raw_os_error());
     }
-
-    #[test]
-    fn parse_ack() {
-        let bytes = vec![0, 0, 0, 0];
-        let msg = ErrorBuffer::new_checked(&bytes)
-            .and_then(|buf| ErrorMessage::parse(&buf))
-            .expect("failed to parse NLMSG_ERROR");
-        assert_eq!(
-            ErrorMessage {
-                code: None,
-                header: Vec::new()
-            },
-            msg
-        );
-        assert_eq!(msg.raw_code(), 0);
-    }
-
-    #[test]
-    fn parse_nack() {
-        // SAFETY: value is non-zero.
-        const ERROR_CODE: NonZeroI32 =
-            unsafe { NonZeroI32::new_unchecked(-1234) };
-        let mut bytes = vec![0, 0, 0, 0];
-        NativeEndian::write_i32(&mut bytes, ERROR_CODE.get());
-        let msg = ErrorBuffer::new_checked(&bytes)
-            .and_then(|buf| ErrorMessage::parse(&buf))
-            .expect("failed to parse NLMSG_ERROR");
-        assert_eq!(
-            ErrorMessage {
-                code: Some(ERROR_CODE),
-                header: Vec::new()
-            },
-            msg
-        );
-        assert_eq!(msg.raw_code(), ERROR_CODE.get());
-    }
 }

src/message.rs

@@ -7,7 +7,7 @@ use netlink_packet_utils::DecodeError;
 
 use crate::{
     payload::{NLMSG_DONE, NLMSG_ERROR, NLMSG_NOOP, NLMSG_OVERRUN},
-    DoneBuffer, DoneMessage, Emitable, ErrorBuffer, ErrorMessage,
+    AckMessage, DoneBuffer, DoneMessage, Emitable, ErrorBuffer, ErrorMessage,
     NetlinkBuffer, NetlinkDeserializable, NetlinkHeader, NetlinkPayload,
     NetlinkSerializable, Parseable,
 };
@@ -101,7 +101,11 @@ where
                 let msg = ErrorBuffer::new_checked(&bytes)
                     .and_then(|buf| ErrorMessage::parse(&buf))
                     .context("failed to parse NLMSG_ERROR")?;
-                Error(msg)
+                if msg.code >= 0 {
+                    Ack(msg as AckMessage)
+                } else {
+                    Error(msg)
+                }
             }
             NLMSG_NOOP => Noop,
             NLMSG_DONE => {
@@ -134,6 +138,7 @@ where
             Done(ref msg) => msg.buffer_len(),
             Overrun(ref bytes) => bytes.len(),
             Error(ref msg) => msg.buffer_len(),
+            Ack(ref msg) => msg.buffer_len(),
             InnerMessage(ref msg) => msg.buffer_len(),
         };
 
@@ -152,6 +157,7 @@ where
             Done(ref msg) => msg.emit(buffer),
             Overrun(ref bytes) => buffer.copy_from_slice(bytes),
             Error(ref msg) => msg.emit(buffer),
+            Ack(ref msg) => msg.emit(buffer),
             InnerMessage(ref msg) => msg.serialize(buffer),
         }
     }
@@ -173,7 +179,7 @@ where
 mod tests {
     use super::*;
 
-    use std::{convert::Infallible, mem::size_of, num::NonZeroI32};
+    use std::{convert::Infallible, mem::size_of};
 
     #[derive(Clone, Debug, Default, PartialEq)]
     struct FakeNetlinkInnerMessage;
@@ -234,34 +240,4 @@ mod tests {
         let got = NetlinkMessage::parse(&NetlinkBuffer::new(&buf)).unwrap();
         assert_eq!(got, want);
     }
-
-    #[test]
-    fn test_error() {
-        // SAFETY: value is non-zero.
-        const ERROR_CODE: NonZeroI32 =
-            unsafe { NonZeroI32::new_unchecked(-8765) };
-
-        let header = NetlinkHeader::default();
-        let error_msg = ErrorMessage {
-            code: Some(ERROR_CODE),
-            header: vec![],
-        };
-        let mut want = NetlinkMessage::new(
-            header,
-            NetlinkPayload::<FakeNetlinkInnerMessage>::Error(error_msg.clone()),
-        );
-        want.finalize();
-
-        let len = want.buffer_len();
-        assert_eq!(len, header.buffer_len() + error_msg.buffer_len());
-
-        let mut buf = vec![1; len];
-        want.emit(&mut buf);
-
-        let error_buf = ErrorBuffer::new(&buf[header.buffer_len()..]);
-        assert_eq!(error_buf.code(), error_msg.code);
-
-        let got = NetlinkMessage::parse(&NetlinkBuffer::new(&buf)).unwrap();
-        assert_eq!(got, want);
-    }
 }

src/payload.rs

@@ -2,7 +2,7 @@
 
 use std::fmt::Debug;
 
-use crate::{DoneMessage, ErrorMessage, NetlinkSerializable};
+use crate::{AckMessage, DoneMessage, ErrorMessage, NetlinkSerializable};
 
 /// The message is ignored.
 pub const NLMSG_NOOP: u16 = 1;
@@ -20,6 +20,7 @@ pub const NLMSG_ALIGNTO: u16 = 4;
 pub enum NetlinkPayload<I> {
     Done(DoneMessage),
     Error(ErrorMessage),
+    Ack(AckMessage),
     Noop,
     Overrun(Vec<u8>),
     InnerMessage(I),
@@ -32,7 +33,7 @@ where
     pub fn message_type(&self) -> u16 {
         match self {
             NetlinkPayload::Done(_) => NLMSG_DONE,
-            NetlinkPayload::Error(_) => NLMSG_ERROR,
+            NetlinkPayload::Error(_) | NetlinkPayload::Ack(_) => NLMSG_ERROR,
             NetlinkPayload::Noop => NLMSG_NOOP,
             NetlinkPayload::Overrun(_) => NLMSG_OVERRUN,
             NetlinkPayload::InnerMessage(message) => message.message_type(),