From 90e9ce0c28f054f9b17ae2acb382939385fc77ed Mon Sep 17 00:00:00 2001
From: Tom Cherry <tomcherry@google.com>
Date: Mon, 1 Jun 2020 13:43:50 -0700
Subject: [PATCH] logd: fix bug in FlushTo when requesting exact sequence
 number

SimpleLogBuffer::FlushTo() attempts to find the iterator matching a
given sequence number, but the logic is wrong and will always skip one
element forward.  This change fixes this and adds a test for the
situation.

This likely contributed to some test instability in the past, but was
identified because subsequent changes that track the start value
closer exacerbated this issue.

Test: existing and new unit tests
Change-Id: Iba2e654e94234693dba20d4747a60bc79d195673
---
 logd/LogBufferTest.cpp   | 35 +++++++++++++++++++++++++++++++++++
 logd/SimpleLogBuffer.cpp |  4 +++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/logd/LogBufferTest.cpp b/logd/LogBufferTest.cpp
index 334d57b60..457d2fb29 100644
--- a/logd/LogBufferTest.cpp
+++ b/logd/LogBufferTest.cpp
@@ -335,4 +335,39 @@ TEST_P(LogBufferTest, random_messages) {
     CompareLogMessages(log_messages, read_log_messages);
 }
 
+TEST_P(LogBufferTest, read_last_sequence) {
+    std::vector<LogMessage> log_messages = {
+            {{.pid = 1, .tid = 2, .sec = 10000, .nsec = 20001, .lid = LOG_ID_MAIN, .uid = 0},
+             "first"},
+            {{.pid = 10, .tid = 2, .sec = 10000, .nsec = 20002, .lid = LOG_ID_MAIN, .uid = 0},
+             "second"},
+            {{.pid = 100, .tid = 2, .sec = 10000, .nsec = 20003, .lid = LOG_ID_MAIN, .uid = 0},
+             "third"},
+    };
+    FixupMessages(&log_messages);
+    LogMessages(log_messages);
+
+    std::vector<LogMessage> read_log_messages;
+    bool released = false;
+
+    {
+        auto lock = std::unique_lock{reader_list_.reader_threads_lock()};
+        std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
+        std::unique_ptr<LogReaderThread> log_reader(
+                new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
+                                    0, ~0, 0, {}, 3, {}));
+        reader_list_.reader_threads().emplace_back(std::move(log_reader));
+    }
+
+    while (!released) {
+        usleep(5000);
+    }
+    {
+        auto lock = std::unique_lock{reader_list_.reader_threads_lock()};
+        EXPECT_EQ(0U, reader_list_.reader_threads().size());
+    }
+    std::vector<LogMessage> expected_log_messages = {log_messages.back()};
+    CompareLogMessages(expected_log_messages, read_log_messages);
+}
+
 INSTANTIATE_TEST_CASE_P(LogBufferTests, LogBufferTest, testing::Values("chatty", "simple"));
diff --git a/logd/SimpleLogBuffer.cpp b/logd/SimpleLogBuffer.cpp
index 8a11b929b..ceecc6d10 100644
--- a/logd/SimpleLogBuffer.cpp
+++ b/logd/SimpleLogBuffer.cpp
@@ -126,7 +126,9 @@ uint64_t SimpleLogBuffer::FlushTo(
         for (it = logs_.end(); it != logs_.begin();
              /* do nothing */) {
             --it;
-            if (it->getSequence() <= start) {
+            if (it->getSequence() == start) {
+                break;
+            } else if (it->getSequence() < start) {
                 it++;
                 break;
             }