Refactor QMDL and DiagReader for better testing

For the QMDL code, this mostly rewrites the structs to accept generic
types that implement Read and Write, which allows for better unit
testing. Also adds a bunch of unit tests.

src/diag.rs

@@ -49,7 +49,7 @@ pub enum DataType {
     Other(u32),
 }
 
-#[derive(Debug, Clone, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 pub struct MessagesContainer {
     pub data_type: DataType,
     pub num_messages: u32,
@@ -57,14 +57,14 @@ pub struct MessagesContainer {
     pub messages: Vec<HdlcEncapsulatedMessage>,
 }
 
-#[derive(Debug, Clone, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 pub struct HdlcEncapsulatedMessage {
     pub len: u32,
     #[deku(count = "len")]
     pub data: Vec<u8>,
 }
 
-#[derive(Debug, Clone, PartialEq, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 #[deku(type = "u8")]
 pub enum Message {
     #[deku(id = "16")]
@@ -93,7 +93,7 @@ pub enum Message {
     },
 }
 
-#[derive(Debug, Clone, PartialEq, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 #[deku(ctx = "log_type: u16, hdr_len: u16", id = "log_type")]
 pub enum LogBody {
     #[deku(id = "0x412f")]
@@ -161,7 +161,7 @@ pub enum LogBody {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 #[deku(ctx = "ext_header_version: u8", id = "ext_header_version")]
 pub enum LteRrcOtaPacket {
     #[deku(id_pat = "0..=4")]
@@ -268,7 +268,7 @@ impl LteRrcOtaPacket {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 #[deku(endian = "little")]
 pub struct Timestamp {
     pub ts: u64,
@@ -288,14 +288,14 @@ impl Timestamp {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 #[deku(ctx = "opcode: u32, subopcode: u32", id = "opcode")]
 pub enum ResponsePayload {
     #[deku(id = "115")]
     LogConfig(#[deku(ctx = "subopcode")] LogConfigResponse),
 }
 
-#[derive(Debug, Clone, PartialEq, DekuRead)]
+#[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
 #[deku(ctx = "subopcode: u32", id = "subopcode")]
 pub enum LogConfigResponse {
     #[deku(id = "1")]

src/diag_device.rs

@@ -1,7 +1,7 @@
 use crate::hdlc::hdlc_encapsulate;
 use crate::diag::{Message, ResponsePayload, Request, LogConfigRequest, LogConfigResponse, build_log_mask_request, RequestContainer, DataType, MessagesContainer};
 use crate::diag_reader::{DiagReader, CRC_CCITT};
-use crate::qmdl::QmdlFileWriter;
+use crate::qmdl::QmdlWriter;
 use crate::log_codes;
 
 use std::fs::File;
@@ -63,7 +63,8 @@ const DIAG_IOCTL_SWITCH_LOGGING: u32 = 7;
 
 pub struct DiagDevice {
     file: File,
-    pub qmdl_file: QmdlFileWriter,
+    pub qmdl_writer: QmdlWriter<File>,
+    fully_initialized: bool,
     read_buf: Vec<u8>,
     use_mdm: i32,
 }
@@ -82,8 +83,11 @@ impl DiagReader for DiagDevice {
         if leftover_bytes.len() > 0 {
             warn!("warning: {} leftover bytes when parsing MessagesContainer", leftover_bytes.len());
         }
-        self.qmdl_file.write_container(&container)
+
-            .map_err(DiagDeviceError::QmdlFileWriteError)?;
+        if self.fully_initialized {
+            self.qmdl_writer.write_container(&container)
+                .map_err(DiagDeviceError::QmdlFileWriteError)?;
+        }
         Ok(container)
     }
 }
@@ -97,8 +101,20 @@ impl DiagDevice {
             .map_err(DiagDeviceError::OpenDiagDeviceError)?;
         let fd = diag_file.as_raw_fd();
 
-        let qmdl_file = QmdlFileWriter::new(qmdl_path)
+        let qmdl_file = File::options()
+            .create(true)
+            .append(true)
+            .open(&qmdl_path)
             .map_err(DiagDeviceError::OpenQmdlFileError)?;
+        let qmdl_metadata = qmdl_file.metadata().map_err(DiagDeviceError::OpenQmdlFileError)?;
+        if qmdl_metadata.len() != 0 {
+            info!(
+                "QMDL file {} already contains data ({} bytes), appending to it",
+                qmdl_path.as_ref().display(),
+                qmdl_metadata.len()
+            );
+        }
+        let qmdl_writer = QmdlWriter::new_with_existing_size(qmdl_file, qmdl_metadata.len() as usize);
 
         enable_frame_readwrite(fd, MEMORY_DEVICE_MODE)?;
         let use_mdm = determine_use_mdm(fd)?;
@@ -106,7 +122,8 @@ impl DiagDevice {
         Ok(DiagDevice {
             read_buf: vec![0; BUFFER_LEN],
             file: diag_file,
-            qmdl_file,
+            fully_initialized: false,
+            qmdl_writer,
             use_mdm,
         })
     }
@@ -187,6 +204,7 @@ impl DiagDevice {
             }
         }
 
+        self.fully_initialized = true;
         Ok(())
     }
 }

src/diag_reader.rs

@@ -21,7 +21,7 @@ pub const CRC_CCITT_ALG: Algorithm<u16> = Algorithm {
 };
 pub const CRC_CCITT: Crc<u16> = Crc::<u16>::new(&CRC_CCITT_ALG);
 
-#[derive(Debug, Error)]
+#[derive(Debug, PartialEq, Error)]
 pub enum DiagParsingError {
     #[error("Failed to parse Message: {0}, data: {1:?}")]
     MessageParsingError(deku::DekuError, Vec<u8>),
@@ -72,3 +72,152 @@ pub trait DiagReader {
         Ok(result)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    struct MockReader {
+        containers: Vec<MessagesContainer>,
+    }
+
+    impl DiagReader for MockReader {
+        type Err = ();
+
+        fn get_next_messages_container(&mut self) -> Result<MessagesContainer, Self::Err> {
+            Ok(self.containers.remove(0))
+        }
+    }
+
+    fn make_container(data_type: DataType, message: HdlcEncapsulatedMessage) -> MessagesContainer {
+        MessagesContainer {
+            data_type,
+            num_messages: 1,
+            messages: vec![message],
+        }
+    }
+
+    // this log is based on one captured on a real device -- if it fails to
+    // serialize or deserialize, that's probably a problem with this mock, not
+    // the DiagReader implementation
+    fn get_test_message(payload: &[u8]) -> (HdlcEncapsulatedMessage, Message) {
+        let length_with_payload = 31 + payload.len() as u16;
+        let message = Message::Log {
+            pending_msgs: 0,
+            outer_length: length_with_payload,
+            inner_length: length_with_payload,
+            log_type: 0xb0c0,
+            timestamp: Timestamp { ts: 72659535985485082 },
+            body: LogBody::LteRrcOtaMessage {
+                ext_header_version: 20,
+                packet: LteRrcOtaPacket::V8 {
+                    rrc_rel_maj: 14,
+                    rrc_rel_min: 48,
+                    bearer_id: 0,
+                    phy_cell_id: 160,
+                    earfcn: 2050,
+                    sfn_subfn: 4057,
+                    pdu_num: 5,
+                    sib_mask: 0,
+                    len: payload.len() as u16,
+                    packet: payload.to_vec(),
+                },
+            },
+        };
+        let serialized = message.to_bytes().expect("failed to serialize test message");
+        let encapsulated_data = hdlc::hdlc_encapsulate(&serialized, &CRC_CCITT);
+        let encapsulated = HdlcEncapsulatedMessage {
+            len: encapsulated_data.len() as u32,
+            data: encapsulated_data,
+        };
+        (encapsulated, message)
+    }
+
+    #[test]
+    fn test_skipping_nonuser_containers() {
+        let (encapsulated1, message1) = get_test_message(&[1]);
+        let (encapsulated2, _) = get_test_message(&[2]);
+        let (encapsulated3, message3) = get_test_message(&[3]);
+        let mut reader = MockReader {
+            containers: vec![
+                make_container(DataType::UserSpace, encapsulated1),
+                make_container(DataType::Other(0), encapsulated2),
+                make_container(DataType::UserSpace, encapsulated3),
+            ],
+        };
+        assert_eq!(reader.read_response(), Ok(vec![Ok(message1)]));
+        assert_eq!(reader.read_response(), Ok(vec![Ok(message3)]));
+    }
+
+    #[test]
+    fn test_containers_with_multiple_messages() {
+        let (encapsulated1, message1) = get_test_message(&[1]);
+        let (encapsulated2, message2) = get_test_message(&[2]);
+        let mut container1 = make_container(DataType::UserSpace, encapsulated1);
+        container1.messages.push(encapsulated2);
+        container1.num_messages += 1;
+        let (encapsulated3, message3) = get_test_message(&[3]);
+        let mut reader = MockReader {
+            containers: vec![
+                container1,
+                make_container(DataType::UserSpace, encapsulated3),
+            ],
+        };
+        assert_eq!(reader.read_response(), Ok(vec![Ok(message1), Ok(message2)]));
+        assert_eq!(reader.read_response(), Ok(vec![Ok(message3)]));
+    }
+
+    #[test]
+    fn test_containers_with_concatenated_message() {
+        let (mut encapsulated1, message1) = get_test_message(&[1]);
+        let (encapsulated2, message2) = get_test_message(&[2]);
+        encapsulated1.data.extend(encapsulated2.data);
+        encapsulated1.len += encapsulated2.len;
+        let (encapsulated3, message3) = get_test_message(&[3]);
+        let mut reader = MockReader {
+            containers: vec![
+                make_container(DataType::UserSpace, encapsulated1),
+                make_container(DataType::UserSpace, encapsulated3),
+            ],
+        };
+        assert_eq!(reader.read_response(), Ok(vec![Ok(message1), Ok(message2)]));
+        assert_eq!(reader.read_response(), Ok(vec![Ok(message3)]));
+    }
+
+    #[test]
+    fn test_handles_parsing_errors() {
+        let (encapsulated1, message1) = get_test_message(&[1]);
+        let bad_message = hdlc::hdlc_encapsulate(&[0x01, 0x02, 0x03, 0x04], &CRC_CCITT);
+        let encapsulated2 = HdlcEncapsulatedMessage {
+            len: bad_message.len() as u32,
+            data: bad_message,
+        };
+        let mut container = make_container(DataType::UserSpace, encapsulated1);
+        container.messages.push(encapsulated2);
+        container.num_messages += 1;
+        let mut reader = MockReader {
+            containers: vec![container],
+        };
+        let result = reader.read_response().unwrap();
+        assert_eq!(result[0], Ok(message1));
+        assert!(matches!(result[1], Err(DiagParsingError::MessageParsingError(_, _))));
+    }
+
+    #[test]
+    fn test_handles_encapsulation_errors() {
+        let (encapsulated1, message1) = get_test_message(&[1]);
+        let bad_encapsulation = HdlcEncapsulatedMessage {
+            len: 4,
+            data: vec![0x01, 0x02, 0x03, 0x04],
+        };
+        let mut container = make_container(DataType::UserSpace, encapsulated1);
+        container.messages.push(bad_encapsulation);
+        container.num_messages += 1;
+        let mut reader = MockReader {
+            containers: vec![container],
+        };
+        let result = reader.read_response().unwrap();
+        assert_eq!(result[0], Ok(message1));
+        assert!(matches!(result[1], Err(DiagParsingError::HdlcDecapsulationError(_, _))));
+    }
+}

src/qmdl.rs

@@ -1,60 +1,81 @@
-//! QMDL files are Qualcomm Mobile Diagnostic Logs. Their format is very simple,
+//! Qualcomm Mobile Diagnostic Log (QMDL) files have a very simple format: just
-//! just a series of of concatenated HDLC encapsulated diag::Message structs.
+//! a series of of concatenated HDLC encapsulated diag::Message structs.
+//! QmdlReader and QmdlWriter can read and write MessagesContainers to and from
+//! QMDL files.
 
 use crate::diag_reader::DiagReader;
 use crate::diag::{MessagesContainer, MESSAGE_TERMINATOR, HdlcEncapsulatedMessage, DataType};
 
-use std::fs::File;
+use std::io::{Write, BufReader, BufRead, Read};
-use std::io::{Write, BufReader, BufRead};
+use thiserror::Error;
+use log::error;
 
-pub struct QmdlFileWriter {
+pub struct QmdlWriter<T> where T: Write {
-    file: File,
+    writer: T,
     pub total_written: usize,
 }
 
-impl QmdlFileWriter {
+impl<T> QmdlWriter<T> where T: Write {
-    pub fn new<P>(path: P) -> std::io::Result<Self> where P: AsRef<std::path::Path> {
+    pub fn new(writer: T) -> Self {
-        let file = std::fs::File::options()
+        QmdlWriter::new_with_existing_size(writer, 0)
-            .create(true)
+    }
-            .append(true)
+
-            .open(path)?;
+    pub fn new_with_existing_size(writer: T, existing_size: usize) -> Self {
-        Ok(QmdlFileWriter {
+        QmdlWriter {
-            file,
+            writer,
-            total_written: 0,
+            total_written: existing_size,
-        })
+        }
     }
 
     pub fn write_container(&mut self, container: &MessagesContainer) -> std::io::Result<()> {
         for msg in &container.messages {
-            self.file.write_all(&msg.data)?;
+            self.writer.write_all(&msg.data)?;
             self.total_written += msg.data.len();
         }
         Ok(())
     }
 }
 
-pub struct QmdlFileReader {
+#[derive(Debug, Error)]
-    file: BufReader<File>,
+pub enum QmdlReaderError {
-    buf: Vec<u8>
+    #[error("IO error: {0}")]
+    IoError(#[from] std::io::Error),
+    #[error("Reached max_bytes count {0}")]
+    MaxBytesReached(usize),
 }
 
-impl QmdlFileReader {
+pub struct QmdlReader<T> where T: Read {
-    pub fn new<P>(path: P) -> std::io::Result<Self> where P: AsRef<std::path::Path> {
+    reader: BufReader<T>,
-        let file = std::fs::File::options()
+    bytes_read: usize,
-            .read(true)
+    max_bytes: Option<usize>,
-            .open(path)?;
+}
-        Ok(QmdlFileReader {
+
-            file: BufReader::new(file),
+impl<T> QmdlReader<T> where T: Read {
-            buf: Vec::new(),
+    pub fn new(reader: T, max_bytes: Option<usize>) -> Self {
-        })
+        QmdlReader {
+            reader: BufReader::new(reader),
+            bytes_read: 0,
+            max_bytes,
+        }
     }
 }
 
-impl DiagReader for QmdlFileReader {
+impl<T> DiagReader for QmdlReader<T> where T: Read {
-    type Err = std::io::Error;
+    type Err = QmdlReaderError;
 
-    fn get_next_messages_container(&mut self) -> std::io::Result<MessagesContainer> {
+    fn get_next_messages_container(&mut self) -> Result<MessagesContainer, Self::Err> {
-        let bytes_read = self.file.read_until(MESSAGE_TERMINATOR, &mut self.buf)?;
+        if let Some(max_bytes) = self.max_bytes {
+            if self.bytes_read >= max_bytes {
+                if self.bytes_read > max_bytes {
+                    error!("warning: {} bytes read, but max_bytes was {}", self.bytes_read, max_bytes);
+                }
+                return Err(QmdlReaderError::MaxBytesReached(max_bytes));
+            }
+        }
+
+        let mut buf = Vec::new();
+        let bytes_read = self.reader.read_until(MESSAGE_TERMINATOR, &mut buf)?;
+        self.bytes_read += bytes_read;
 
         // Since QMDL is just a flat list of messages, we can't actually
         // reproduce the container structure they came from in the original
@@ -66,10 +87,126 @@ impl DiagReader for QmdlFileReader {
             num_messages: 1,
             messages: vec![
                 HdlcEncapsulatedMessage {
-                    len: 1,
+                    len: bytes_read as u32,
-                    data: self.buf[0..bytes_read].to_vec(),
+                    data: buf,
                 },
             ]
         })
     }
 }
+
+#[cfg(test)]
+mod test {
+    use std::io::Cursor;
+
+    use crate::hdlc::hdlc_encapsulate;
+    use crate::diag_reader::CRC_CCITT;
+
+    use super::*;
+
+    fn get_test_messages() -> Vec<HdlcEncapsulatedMessage> {
+        let messages: Vec<HdlcEncapsulatedMessage> = (10..20).map(|i| {
+            let data = hdlc_encapsulate(&vec![i as u8; i], &CRC_CCITT);
+            HdlcEncapsulatedMessage {
+                len: data.len() as u32,
+                data,
+            }
+        }).collect();
+        messages
+    }
+
+    // returns a byte array consisting of concatenated HDLC encapsulated
+    // test messages
+    fn get_test_message_bytes() -> Vec<u8> {
+        get_test_messages().iter()
+            .flat_map(|msg| msg.data.clone())
+            .collect()
+    }
+
+    fn get_test_containers() -> Vec<MessagesContainer> {
+        let messages = get_test_messages();
+        let (messages1, messages2) = messages.split_at(5);
+        vec![
+            MessagesContainer {
+                data_type: DataType::UserSpace,
+                num_messages: messages1.len() as u32,
+                messages: messages1.to_vec(),
+            },
+            MessagesContainer {
+                data_type: DataType::UserSpace,
+                num_messages: messages2.len() as u32,
+                messages: messages2.to_vec()
+            },
+        ]
+    }
+
+    #[test]
+    fn test_unbounded_qmdl_reader() {
+        let mut buf = Cursor::new(get_test_message_bytes());
+        let mut reader = QmdlReader::new(&mut buf, None);
+        let expected_messages = get_test_messages();
+        for message in expected_messages {
+            let expected_container = MessagesContainer {
+                data_type: DataType::UserSpace,
+                num_messages: 1,
+                messages: vec![message],
+            };
+            assert_eq!(expected_container, reader.get_next_messages_container().unwrap());
+        }
+    }
+
+    #[test]
+    fn test_bounded_qmdl_reader() {
+        let mut buf = Cursor::new(get_test_message_bytes());
+
+        // bound the reader to the first two messages
+        let mut expected_messages = get_test_messages();
+        let limit = expected_messages[0].len + expected_messages[1].len;
+
+        let mut reader = QmdlReader::new(&mut buf, Some(limit as usize));
+        for message in expected_messages.drain(0..2) {
+            let expected_container = MessagesContainer {
+                data_type: DataType::UserSpace,
+                num_messages: 1,
+                messages: vec![message],
+            };
+            assert_eq!(expected_container, reader.get_next_messages_container().unwrap());
+        }
+        assert!(matches!(reader.get_next_messages_container(), Err(QmdlReaderError::MaxBytesReached(_))));
+    }
+
+    #[test]
+    fn test_qmdl_writer() {
+        let mut buf = Vec::new();
+        let mut writer = QmdlWriter::new(&mut buf);
+        let expected_containers = get_test_containers();
+        for container in &expected_containers {
+            writer.write_container(container).unwrap();
+        }
+        assert_eq!(writer.total_written, buf.len());
+        assert_eq!(buf, get_test_message_bytes());
+    }
+
+    #[test]
+    fn test_writing_and_reading() {
+        let mut buf = Vec::new();
+        let mut writer = QmdlWriter::new(&mut buf);
+        let expected_containers = get_test_containers();
+        for container in &expected_containers {
+            writer.write_container(container).unwrap();
+        }
+
+        let limit = Some(buf.len());
+        let mut reader = QmdlReader::new(Cursor::new(&mut buf), limit);
+        let expected_messages = get_test_messages();
+        for message in expected_messages {
+            let expected_container = MessagesContainer {
+                data_type: DataType::UserSpace,
+                num_messages: 1,
+                messages: vec![message],
+            };
+            assert_eq!(expected_container, reader.get_next_messages_container().unwrap());
+        }
+        assert!(matches!(reader.get_next_messages_container(), Err(QmdlReaderError::MaxBytesReached(_))));
+    }
+}