From 241e581359f381c5aaa18cefa3dbcb7d46241267 Mon Sep 17 00:00:00 2001
From: Tilman Sauerbeck <tilman@code-monkey.de>
Date: Sun, 12 Jan 2020 12:56:56 +0100
Subject: [PATCH] common: Avoid duplicated recordings shown in
 Logger::list_recordings().

This is achieved by simplifying the way we sort our sector indices,
which unfortunately also comes with a performance hit.
---
 src/common/logger.rs | 67 +++++++++++---------------------------------
 test/logger_test.rs  | 52 ++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 50 deletions(-)

diff --git a/src/common/logger.rs b/src/common/logger.rs
index 95f4a43..2ca2e22 100644
--- a/src/common/logger.rs
+++ b/src/common/logger.rs
@@ -144,10 +144,22 @@ fn cmp_sector_header_indices(a: u16, b: u16,
     let header_a = &sector_header[a as usize];
     let header_b = &sector_header[b as usize];
 
-    // Latest entries come first.
-    if header_a.start_time > header_b.start_time {
+    if header_a.starts_recording() && header_b.starts_recording() {
+        // Latest entries come first.
+        if header_a.start_time > header_b.start_time {
+            -1
+        } else if header_a.start_time < header_b.start_time {
+            1
+        } else {
+            0
+        }
+    } else if header_a.starts_recording() {
+        -1
+    } else if header_b.starts_recording() {
+        1
+    } else if a < b {
         -1
-    } else if header_a.start_time < header_b.start_time {
+    } else if a > b {
         1
     } else {
         0
@@ -187,56 +199,11 @@ impl<'a> SectorHeaderIter<'a> {
             indices: [0; NUM_SECTORS]
         };
 
-        let mut num_used = 0;
-
-        // Put the indices of the used directory entries at the beginning
-        // of the array. Ignore the unused ones since we are not going
-        // to sort them anyway.
         for i in 0..NUM_SECTORS {
-            let sector_header = &iter.sector_header[i];
-
-            if sector_header.starts_recording() {
-                iter.indices[num_used] = i as u16;
-                num_used += 1;
-            }
+            iter.indices[i] = i as u16;
         }
 
-        let num_elts_to_sort = num_used;
-
-        if num_elts_to_sort != 0 {
-            // Sort the used directory entries.
-            iter.sort(num_elts_to_sort);
-        }
-
-        // Now put the indices of the unused directory entries in the array.
-        if num_used == 0 {
-            for i in 0..NUM_SECTORS {
-                iter.indices[i] = i as u16;
-            }
-        } else {
-            let latest_used = iter.indices[0] as usize;
-            let mut offset_unused = num_used;
-
-            // First put the entries that come after the latest one in use...
-            for i in (latest_used + 1)..NUM_SECTORS {
-                let sector_header = &iter.sector_header[i];
-
-                if !sector_header.is_in_use() {
-                    iter.indices[offset_unused] = i as u16;
-                    offset_unused += 1;
-                }
-            }
-
-            // ... then wrap around if necessary.
-            for i in 0..latest_used {
-                let sector_header = &iter.sector_header[i];
-
-                if !sector_header.is_in_use() {
-                    iter.indices[offset_unused] = i as u16;
-                    offset_unused += 1;
-                }
-            }
-        }
+        iter.sort(NUM_SECTORS);
 
         // XXX:
         // Need to handle those sectors that don't start recordings
diff --git a/test/logger_test.rs b/test/logger_test.rs
index bbf8dd5..b110055 100644
--- a/test/logger_test.rs
+++ b/test/logger_test.rs
@@ -430,3 +430,55 @@ fn list_recordings1() {
     assert_eq!(String::from_utf8(expected_bytes.to_vec()).unwrap(),
                String::from_utf8(listing_raw[0..num_bytes_read].to_vec()).unwrap());
 }
+
+// Verifies that Logger::list_recordings() handles sectors holding
+// additional recording data.
+#[test]
+fn list_recording1_multi_sector() {
+    let pipe = Pipe::new();
+
+    let mut file = unsafe { std::fs::File::from_raw_fd(pipe.write_fd()) };
+
+    let mut ls_buffer_space = [0u8; 4096];
+    let mut ls_buffer = Buffer::alloc();
+
+    let user_data = (&mut file as *mut std::fs::File) as *mut BufferUserData;
+
+    ls_buffer.init(ls_buffer_space.as_mut_ptr(),
+                   ls_buffer_space.len(),
+                   flush_write_buffer,
+                   user_data);
+
+    let mut fake_storage = FakeStorage::new();
+
+    // The first sector starts recording 1.
+    let header0 = [
+        // Header:
+        0x01, 0x00, 0x01, 0x00,
+        0x75, 0x18, 0x17, 0x5e,
+    ];
+
+    // The second sector has additional data for recording 1.
+    let header1 = [
+        0x03, 0x00, 0x01, 0x00,
+    ];
+
+    fake_storage.actual[0..header0.len()].copy_from_slice(&header0);
+    fake_storage.actual[4096..4096 + header1.len()].copy_from_slice(&header1);
+
+    let mut logger = Logger::new(&mut fake_storage);
+    logger.init();
+
+    logger.list_recordings(&mut ls_buffer);
+
+    let mut file = unsafe { std::fs::File::from_raw_fd(pipe.read_fd()) };
+
+    let mut listing_raw = [0u8; 4096];
+    let num_bytes_read = file.read(&mut listing_raw).unwrap();
+
+    let expected_bytes = b"\
+2020-01-09 12:11:33              1\n";
+
+    assert_eq!(String::from_utf8(expected_bytes.to_vec()).unwrap(),
+               String::from_utf8(listing_raw[0..num_bytes_read].to_vec()).unwrap());
+}
-- 
2.30.2