diff options
author | Tom Cherry <tomcherry@google.com> | 2020-09-09 20:13:18 +0000 |
---|---|---|
committer | Tom Cherry <tomcherry@google.com> | 2020-09-09 20:26:59 +0000 |
commit | 2adbece108c0f29dd417216173176c016440e1e5 (patch) | |
tree | 7c34accc2a9f07910387cfae3bcaa62daafb1293 /logd/SerializedLogChunkTest.cpp | |
parent | 29e7b2af306c69a3e121bca1830a642eeac64019 (diff) |
logd: Fix ClearUidLogs() when writer_active_ is true
Previously ClearUidLogs() would Compress() the log buffer in all
cases, however that is the wrong behavior when writer_active_ is true
and would leave the SerializedLogChunk object in an invalid state. If
more logs are written to the log, then write_offset() will be higher
than the compressed size of the log, violating a CHECK() when later
decompressing the log.
This change does not call Compress() in ClearUidLogs() if
writer_active_ is true. It upgrades a check in Compress() from a
simple if statement to a CHECK() to prevent against this happening in
the future.
It adds a test that exercises the previously failing path.
Bug: 166187079
Test: unit tests
Change-Id: Ic5fbcf16f724af1c20170b8f4e6e2daadf6a9529
Diffstat (limited to 'logd/SerializedLogChunkTest.cpp')
-rw-r--r-- | logd/SerializedLogChunkTest.cpp | 24 |
1 files changed, 24 insertions, 0 deletions
diff --git a/logd/SerializedLogChunkTest.cpp b/logd/SerializedLogChunkTest.cpp index f10b9c673..3b451252d 100644 --- a/logd/SerializedLogChunkTest.cpp +++ b/logd/SerializedLogChunkTest.cpp @@ -281,3 +281,27 @@ TEST_P(UidClearTest, save_beginning_and_end) { } INSTANTIATE_TEST_CASE_P(UidClearTests, UidClearTest, testing::Values(true, false)); + +// b/166187079: ClearUidLogs() should not compress the log if writer_active_ is true +TEST(SerializedLogChunk, ClearUidLogs_then_FinishWriting) { + static constexpr size_t kChunkSize = 10 * 4096; + static constexpr uid_t kUidToClear = 1000; + static constexpr uid_t kOtherUid = 1234; + + SerializedLogChunk chunk{kChunkSize}; + static const char msg1[] = "saved first message"; + static const char msg2[] = "cleared interior message"; + static const char msg3[] = "last message stays"; + ASSERT_NE(nullptr, chunk.Log(1, log_time(), kOtherUid, 1, 2, msg1, sizeof(msg1))); + ASSERT_NE(nullptr, chunk.Log(2, log_time(), kUidToClear, 1, 2, msg2, sizeof(msg2))); + ASSERT_NE(nullptr, chunk.Log(3, log_time(), kOtherUid, 1, 2, msg3, sizeof(msg3))); + + chunk.ClearUidLogs(kUidToClear, LOG_ID_MAIN, nullptr); + + ASSERT_NE(nullptr, chunk.Log(4, log_time(), kOtherUid, 1, 2, msg3, sizeof(msg3))); + + chunk.FinishWriting(); + // The next line would violate a CHECK() during decompression with the faulty code. + chunk.IncReaderRefCount(); + chunk.DecReaderRefCount(); +} |