diff --git a/current.txt b/current.txt index beacf41f04..5eb7a6f146 100644 --- a/current.txt +++ b/current.txt @@ -613,7 +613,7 @@ dd377f404a8e71f6191d295e10067db629b0f0c28e594af906f2bea5d87fe2cc android.hardwar 07d0a252b2d8fa35887908a996ba395cf392968395fc30afab791f46e0c22a52 android.hardware.boot@1.1::IBootControl 74049a402be913963edfdd80828a53736570e9d8124a1bf18166b6ed46a6b0ab android.hardware.boot@1.1::types d9df99be0f59d8f33a9699fe316c67bfd11818aa69440bb1123ba43e717cea85 android.hardware.dumpstate@1.1::types -f284ffde7cadf5a1364b75ab313baf22401eeca289bdde2a2dc7a27ea4ab98d7 android.hardware.dumpstate@1.1::IDumpstateDevice +186bc152ae189ab48f3a761a44ddf5edd0d483073c5b6ca1f802f8b50488b754 android.hardware.dumpstate@1.1::IDumpstateDevice ce8dbe76eb9ee94b46ef98f725be992e760a5751073d4f4912484026541371f3 android.hardware.health@2.1::IHealth 26f04510a0b57aba5167c5c0a7c2f077c2acbb98b81902a072517829fd9fd67f android.hardware.health@2.1::IHealthInfoCallback e2f8bc1868fd4a3fd587c172773ea5a8c2f5a3deaf7958394102ca455252b255 android.hardware.health@2.1::types diff --git a/dumpstate/1.1/IDumpstateDevice.hal b/dumpstate/1.1/IDumpstateDevice.hal index 502c460998..183404dd68 100644 --- a/dumpstate/1.1/IDumpstateDevice.hal +++ b/dumpstate/1.1/IDumpstateDevice.hal @@ -29,8 +29,9 @@ interface IDumpstateDevice extends @1.0::IDumpstateDevice { * The 1.0 version of #dumpstateBoard(handle) should just delegate to this new method and pass * DumpstateMode::DEFAULT and a timeout of 30,000ms (30 seconds). * - * This method may still be called by the dumpstate routine even if getDeviceLoggingEnabled - * returns false. In this case, it must return DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED. + * This method may still be called by the dumpstate routine even if getVerboseLoggingEnabled + * returns false. In this case, it may include essential information but must not include + * information that identifies the user. * * @param h A native handle with one or two valid file descriptors. The first FD is for text * output, the second (if present) is for binary output. @@ -44,7 +45,7 @@ interface IDumpstateDevice extends @1.0::IDumpstateDevice { generates (DumpstateStatus status); /** - * Turns device vendor logging on or off. + * Turns verbose device vendor logging on or off. * * The setting should be persistent across reboots. Underlying implementations may need to start * vendor logging daemons, set system properties, or change logging masks, for example. Given @@ -52,20 +53,21 @@ interface IDumpstateDevice extends @1.0::IDumpstateDevice { * memory/storage/battery impacts, calling this method on a user build should only be done after * user consent has been obtained, e.g. from a toggle in developer settings. * - * Even if device logging has been disabled, dumpstateBoard may still be called by the dumpstate - * routine. In this case, it must return DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED. + * Even if verbose logging has been disabled, dumpstateBoard may still be called by the + * dumpstate routine, and essential information that does not identify the user may be included. * - * @param enable Whether to enable or disable device vendor logging. + * @param enable Whether to enable or disable verbose vendor logging. */ - setDeviceLoggingEnabled(bool enable); + setVerboseLoggingEnabled(bool enable); /** - * Queries the current state of device logging. Primarily for UI and informative purposes. + * Queries the current state of verbose device logging. Primarily for UI and informative + * purposes. * - * Even if device logging has been disabled, dumpstateBoard may still be called by the dumpstate - * routine. In this case, it must return DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED. + * Even if verbose logging has been disabled, dumpstateBoard may still be called by the + * dumpstate routine, and essential information that does not identify the user may be included. * - * @return enabled Whether or not vendor logging is currently enabled. + * @return enabled Whether or not verbose vendor logging is currently enabled. */ - getDeviceLoggingEnabled() generates (bool enabled); + getVerboseLoggingEnabled() generates (bool enabled); }; diff --git a/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp b/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp index 1811b7312b..cbdd87bc78 100644 --- a/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp +++ b/dumpstate/1.1/vts/functional/VtsHalDumpstateV1_1TargetTest.cpp @@ -48,8 +48,8 @@ class DumpstateHidl1_1Test : public ::testing::TestWithParam { ASSERT_NE(dumpstate, nullptr) << "Could not get HIDL instance"; } - void ToggleDeviceLogging(bool enable) { - Return status = dumpstate->setDeviceLoggingEnabled(enable); + void ToggleVerboseLogging(bool enable) { + Return status = dumpstate->setVerboseLoggingEnabled(enable); ASSERT_TRUE(status.isOk()) << "Status should be ok: " << status.description(); if (!dumpstate->ping().isOk()) { @@ -58,11 +58,11 @@ class DumpstateHidl1_1Test : public ::testing::TestWithParam { GetService(); } - Return logging_enabled = dumpstate->getDeviceLoggingEnabled(); + Return logging_enabled = dumpstate->getVerboseLoggingEnabled(); ASSERT_TRUE(logging_enabled.isOk()) << "Status should be ok: " << logging_enabled.description(); ASSERT_EQ(logging_enabled, enable) - << "Device logging should now be " << (enable ? "enabled" : "disabled"); + << "Verbose logging should now be " << (enable ? "enabled" : "disabled"); if (!dumpstate->ping().isOk()) { ALOGW("IDumpstateDevice service appears to have exited lazily, attempting to get " @@ -71,9 +71,9 @@ class DumpstateHidl1_1Test : public ::testing::TestWithParam { } } - void EnableDeviceLogging() { ToggleDeviceLogging(true); } + void EnableVerboseLogging() { ToggleVerboseLogging(true); } - void DisableDeviceLogging() { ToggleDeviceLogging(false); } + void DisableVerboseLogging() { ToggleVerboseLogging(false); } sp dumpstate; }; @@ -123,7 +123,7 @@ void AssertStatusForMode(const DumpstateMode mode, const Return // Negative test: make sure dumpstateBoard() doesn't crash when passed a null pointer. TEST_FOR_ALL_DUMPSTATE_MODES(TestNullHandle, [this](DumpstateMode mode) { - EnableDeviceLogging(); + EnableVerboseLogging(); Return status = dumpstate->dumpstateBoard_1_1(nullptr, mode, kDefaultTimeoutMillis); @@ -133,7 +133,7 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestNullHandle, [this](DumpstateMode mode) { // Negative test: make sure dumpstateBoard() ignores a handle with no FD. TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithNoFd, [this](DumpstateMode mode) { - EnableDeviceLogging(); + EnableVerboseLogging(); native_handle_t* handle = native_handle_create(0, 0); ASSERT_NE(handle, nullptr) << "Could not create native_handle"; @@ -149,7 +149,7 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithNoFd, [this](DumpstateMode mode) { // Positive test: make sure dumpstateBoard() writes something to the FD. TEST_FOR_ALL_DUMPSTATE_MODES(TestOk, [this](DumpstateMode mode) { - EnableDeviceLogging(); + EnableVerboseLogging(); // Index 0 corresponds to the read end of the pipe; 1 to the write end. int fds[2]; @@ -174,7 +174,7 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestOk, [this](DumpstateMode mode) { // Positive test: make sure dumpstateBoard() doesn't crash with two FDs. TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithTwoFds, [this](DumpstateMode mode) { - EnableDeviceLogging(); + EnableVerboseLogging(); int fds1[2]; int fds2[2]; @@ -204,7 +204,7 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestHandleWithTwoFds, [this](DumpstateMode mode) { // Make sure dumpstateBoard_1_1 actually validates its arguments. TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Negative) { - EnableDeviceLogging(); + EnableVerboseLogging(); int fds[2]; ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno; @@ -226,7 +226,7 @@ TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Negative) { } TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Undefined) { - EnableDeviceLogging(); + EnableVerboseLogging(); int fds[2]; ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno; @@ -249,7 +249,7 @@ TEST_P(DumpstateHidl1_1Test, TestInvalidModeArgument_Undefined) { // Positive test: make sure dumpstateBoard() from 1.0 doesn't fail. TEST_P(DumpstateHidl1_1Test, Test1_0MethodOk) { - EnableDeviceLogging(); + EnableVerboseLogging(); int fds[2]; ASSERT_EQ(0, pipe2(fds, O_NONBLOCK)) << errno; @@ -270,9 +270,10 @@ TEST_P(DumpstateHidl1_1Test, Test1_0MethodOk) { native_handle_delete(handle); } -// Make sure disabling device logging behaves correctly. -TEST_FOR_ALL_DUMPSTATE_MODES(TestDeviceLoggingDisabled, [this](DumpstateMode mode) { - DisableDeviceLogging(); +// Make sure disabling verbose logging behaves correctly. Some info is still allowed to be emitted, +// but it can't have privacy/storage/battery impacts. +TEST_FOR_ALL_DUMPSTATE_MODES(TestVerboseLoggingDisabled, [this](DumpstateMode mode) { + DisableVerboseLogging(); // Index 0 corresponds to the read end of the pipe; 1 to the write end. int fds[2]; @@ -285,11 +286,10 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestDeviceLoggingDisabled, [this](DumpstateMode mod Return status = dumpstate->dumpstateBoard_1_1(handle, mode, kDefaultTimeoutMillis); - AssertStatusForMode(mode, status, DumpstateStatus::DEVICE_LOGGING_NOT_ENABLED, [&fds]() { - // Check that nothing was written. Could return 0 or -1. - char buff; - ASSERT_NE(1, read(fds[0], &buff, 1)) << "Dumped something when device logging is disabled"; - }); + // We don't include additional assertions here about the file passed in. If verbose logging is + // disabled, the OEM may choose to include nothing at all, but it is allowed to include some + // essential information based on the mode as long as it isn't private user information. + AssertStatusForMode(mode, status, DumpstateStatus::OK); native_handle_close(handle); native_handle_delete(handle); @@ -297,22 +297,22 @@ TEST_FOR_ALL_DUMPSTATE_MODES(TestDeviceLoggingDisabled, [this](DumpstateMode mod // Double-enable is perfectly valid, but the second call shouldn't do anything. TEST_P(DumpstateHidl1_1Test, TestRepeatedEnable) { - EnableDeviceLogging(); - EnableDeviceLogging(); + EnableVerboseLogging(); + EnableVerboseLogging(); } // Double-disable is perfectly valid, but the second call shouldn't do anything. TEST_P(DumpstateHidl1_1Test, TestRepeatedDisable) { - DisableDeviceLogging(); - DisableDeviceLogging(); + DisableVerboseLogging(); + DisableVerboseLogging(); } // Toggling in short order is perfectly valid. TEST_P(DumpstateHidl1_1Test, TestRepeatedToggle) { - EnableDeviceLogging(); - DisableDeviceLogging(); - EnableDeviceLogging(); - DisableDeviceLogging(); + EnableVerboseLogging(); + DisableVerboseLogging(); + EnableVerboseLogging(); + DisableVerboseLogging(); } INSTANTIATE_TEST_SUITE_P(