Merge "Fix the order for boot scripts scanning"
diff --git a/fs_mgr/TEST_MAPPING b/fs_mgr/TEST_MAPPING
index 6cd0430..a349408 100644
--- a/fs_mgr/TEST_MAPPING
+++ b/fs_mgr/TEST_MAPPING
@@ -7,7 +7,7 @@
       "name": "liblp_test"
     },
     {
-      "name": "fiemap_image_test_presubmit"
+      "name": "fiemap_image_test"
     },
     {
       "name": "fiemap_writer_test"
diff --git a/fs_mgr/libfiemap/Android.bp b/fs_mgr/libfiemap/Android.bp
index cae43e6..a622110 100644
--- a/fs_mgr/libfiemap/Android.bp
+++ b/fs_mgr/libfiemap/Android.bp
@@ -110,30 +110,3 @@
     auto_gen_config: true,
     require_root: true,
 }
-
-/* BUG(148874852) temporary test */
-cc_test {
-    name: "fiemap_image_test_presubmit",
-    cppflags: [
-        "-DSKIP_TEST_IN_PRESUBMIT",
-    ],
-    static_libs: [
-        "libcrypto_utils",
-        "libdm",
-        "libext4_utils",
-        "libfs_mgr",
-        "liblp",
-    ],
-    shared_libs: [
-        "libbase",
-        "libcrypto",
-        "libcutils",
-        "liblog",
-    ],
-    srcs: [
-        "image_test.cpp",
-    ],
-    test_suites: ["device-tests"],
-    auto_gen_config: true,
-    require_root: true,
-}
diff --git a/fs_mgr/libfiemap/image_manager.cpp b/fs_mgr/libfiemap/image_manager.cpp
index 3ee742f..f32e0eb 100644
--- a/fs_mgr/libfiemap/image_manager.cpp
+++ b/fs_mgr/libfiemap/image_manager.cpp
@@ -136,13 +136,13 @@
     return !!FindPartition(*metadata.get(), name);
 }
 
-static bool IsTestDir(const std::string& path) {
-    return android::base::StartsWith(path, kTestImageMetadataDir) ||
-           android::base::StartsWith(path, kOtaTestImageMetadataDir);
+bool ImageManager::MetadataDirIsTest() const {
+    return IsSubdir(metadata_dir_, kTestImageMetadataDir) ||
+           IsSubdir(metadata_dir_, kOtaTestImageMetadataDir);
 }
 
-static bool IsUnreliablePinningAllowed(const std::string& path) {
-    return android::base::StartsWith(path, "/data/gsi/dsu/") || IsTestDir(path);
+bool ImageManager::IsUnreliablePinningAllowed() const {
+    return IsSubdir(data_dir_, "/data/gsi/dsu/") || MetadataDirIsTest();
 }
 
 FiemapStatus ImageManager::CreateBackingImage(
@@ -159,7 +159,7 @@
     if (!FilesystemHasReliablePinning(data_path, &reliable_pinning)) {
         return FiemapStatus::Error();
     }
-    if (!reliable_pinning && !IsUnreliablePinningAllowed(data_path)) {
+    if (!reliable_pinning && !IsUnreliablePinningAllowed()) {
         // For historical reasons, we allow unreliable pinning for certain use
         // cases (DSUs, testing) because the ultimate use case is either
         // developer-oriented or ephemeral (the intent is to boot immediately
@@ -178,7 +178,7 @@
     // if device-mapper is stacked in some complex way not supported by
     // FiemapWriter.
     auto device_path = GetDevicePathForFile(fw.get());
-    if (android::base::StartsWith(device_path, "/dev/block/dm-") && !IsTestDir(metadata_dir_)) {
+    if (android::base::StartsWith(device_path, "/dev/block/dm-") && !MetadataDirIsTest()) {
         LOG(ERROR) << "Cannot persist images against device-mapper device: " << device_path;
 
         fw = {};
diff --git a/fs_mgr/libfiemap/image_test.cpp b/fs_mgr/libfiemap/image_test.cpp
index 6663391..6d09751 100644
--- a/fs_mgr/libfiemap/image_test.cpp
+++ b/fs_mgr/libfiemap/image_test.cpp
@@ -34,10 +34,13 @@
 #include <libdm/dm.h>
 #include <libfiemap/image_manager.h>
 
+#include "utility.h"
+
 using namespace android::dm;
 using namespace std::literals;
 using android::base::unique_fd;
 using android::fiemap::ImageManager;
+using android::fiemap::IsSubdir;
 using android::fs_mgr::BlockDeviceInfo;
 using android::fs_mgr::PartitionOpener;
 using android::fs_mgr::WaitForFile;
@@ -131,6 +134,51 @@
     ASSERT_TRUE(manager_->UnmapImageDevice(base_name_));
 }
 
+namespace {
+
+struct IsSubdirTestParam {
+    std::string child;
+    std::string parent;
+    bool result;
+};
+
+class IsSubdirTest : public ::testing::TestWithParam<IsSubdirTestParam> {};
+
+TEST_P(IsSubdirTest, Test) {
+    const auto& param = GetParam();
+    EXPECT_EQ(param.result, IsSubdir(param.child, param.parent))
+            << "IsSubdir(child=\"" << param.child << "\", parent=\"" << param.parent
+            << "\") != " << (param.result ? "true" : "false");
+}
+
+std::vector<IsSubdirTestParam> IsSubdirTestValues() {
+    // clang-format off
+    std::vector<IsSubdirTestParam> base_cases{
+            {"/foo/bar",     "/foo",     true},
+            {"/foo/bar/baz", "/foo",     true},
+            {"/foo",         "/foo",     true},
+            {"/foo",         "/",        true},
+            {"/",            "/",        true},
+            {"/foo",         "/foo/bar", false},
+            {"/foo",         "/bar",     false},
+            {"/foo-bar",     "/foo",     false},
+            {"/",            "/foo",     false},
+    };
+    // clang-format on
+    std::vector<IsSubdirTestParam> ret;
+    for (const auto& e : base_cases) {
+        ret.push_back(e);
+        ret.push_back({e.child + "/", e.parent, e.result});
+        ret.push_back({e.child, e.parent + "/", e.result});
+        ret.push_back({e.child + "/", e.parent + "/", e.result});
+    }
+    return ret;
+}
+
+INSTANTIATE_TEST_SUITE_P(IsSubdirTest, IsSubdirTest, ::testing::ValuesIn(IsSubdirTestValues()));
+
+}  // namespace
+
 bool Mkdir(const std::string& path) {
     if (mkdir(path.c_str(), 0700) && errno != EEXIST) {
         std::cerr << "Could not mkdir " << path << ": " << strerror(errno) << std::endl;
diff --git a/fs_mgr/libfiemap/include/libfiemap/image_manager.h b/fs_mgr/libfiemap/include/libfiemap/image_manager.h
index 60b98dc..50f4f33 100644
--- a/fs_mgr/libfiemap/include/libfiemap/image_manager.h
+++ b/fs_mgr/libfiemap/include/libfiemap/image_manager.h
@@ -176,6 +176,8 @@
     bool MapWithDmLinear(const IPartitionOpener& opener, const std::string& name,
                          const std::chrono::milliseconds& timeout_ms, std::string* path);
     bool UnmapImageDevice(const std::string& name, bool force);
+    bool IsUnreliablePinningAllowed() const;
+    bool MetadataDirIsTest() const;
 
     ImageManager(const ImageManager&) = delete;
     ImageManager& operator=(const ImageManager&) = delete;
diff --git a/fs_mgr/libfiemap/utility.cpp b/fs_mgr/libfiemap/utility.cpp
index c189855..54cf183 100644
--- a/fs_mgr/libfiemap/utility.cpp
+++ b/fs_mgr/libfiemap/utility.cpp
@@ -167,5 +167,30 @@
     return F2fsPinBeforeAllocate(fd, supported);
 }
 
+bool IsSubdir(const std::string& child, const std::string& parent) {
+    // Precondition: both are absolute paths.
+    CHECK(android::base::StartsWith(child, "/")) << "Not an absolute path: " << child;
+    CHECK(android::base::StartsWith(parent, "/")) << "Not an absolute path: " << parent;
+
+    // Remove extraneous "/" at the end.
+    std::string_view child_sv = child;
+    while (child_sv != "/" && android::base::ConsumeSuffix(&child_sv, "/"))
+        ;
+
+    std::string_view parent_sv = parent;
+    while (parent_sv != "/" && android::base::ConsumeSuffix(&parent_sv, "/"))
+        ;
+
+    // IsSubdir(anything, "/") => true
+    if (parent_sv == "/") return true;
+
+    // IsSubdir("/foo", "/foo") => true
+    if (parent_sv == child_sv) return true;
+
+    // IsSubdir("/foo/bar", "/foo") => true
+    // IsSubdir("/foo-bar", "/foo") => false
+    return android::base::StartsWith(child_sv, std::string(parent_sv) + "/");
+}
+
 }  // namespace fiemap
 }  // namespace android
diff --git a/fs_mgr/libfiemap/utility.h b/fs_mgr/libfiemap/utility.h
index 4c0bc2b..aa40f79 100644
--- a/fs_mgr/libfiemap/utility.h
+++ b/fs_mgr/libfiemap/utility.h
@@ -51,5 +51,9 @@
 // cases (such as snapshots or adb remount).
 bool FilesystemHasReliablePinning(const std::string& file, bool* supported);
 
+// Crude implementation to check if |child| is a subdir of |parent|.
+// Assume both are absolute paths.
+bool IsSubdir(const std::string& child, const std::string& parent);
+
 }  // namespace fiemap
 }  // namespace android
diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h
index 98bf56a..8e369b0 100644
--- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h
+++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h
@@ -180,5 +180,22 @@
     uint64_t bsize_ = 0;
 };
 
+bool IsVirtualAbEnabled();
+
+#define SKIP_IF_NON_VIRTUAL_AB()                                                        \
+    do {                                                                                \
+        if (!IsVirtualAbEnabled()) GTEST_SKIP() << "Test for Virtual A/B devices only"; \
+    } while (0)
+
+#define RETURN_IF_NON_VIRTUAL_AB_MSG(msg) \
+    do {                                  \
+        if (!IsVirtualAbEnabled()) {      \
+            std::cerr << (msg);           \
+            return;                       \
+        }                                 \
+    } while (0)
+
+#define RETURN_IF_NON_VIRTUAL_AB() RETURN_IF_NON_VIRTUAL_AB_MSG("")
+
 }  // namespace snapshot
 }  // namespace android
diff --git a/fs_mgr/libsnapshot/partition_cow_creator_test.cpp b/fs_mgr/libsnapshot/partition_cow_creator_test.cpp
index adfb975..2970222 100644
--- a/fs_mgr/libsnapshot/partition_cow_creator_test.cpp
+++ b/fs_mgr/libsnapshot/partition_cow_creator_test.cpp
@@ -41,8 +41,14 @@
 
 class PartitionCowCreatorTest : public ::testing::Test {
   public:
-    void SetUp() override { SnapshotTestPropertyFetcher::SetUp(); }
-    void TearDown() override { SnapshotTestPropertyFetcher::TearDown(); }
+    void SetUp() override {
+        SKIP_IF_NON_VIRTUAL_AB();
+        SnapshotTestPropertyFetcher::SetUp();
+    }
+    void TearDown() override {
+        RETURN_IF_NON_VIRTUAL_AB();
+        SnapshotTestPropertyFetcher::TearDown();
+    }
 };
 
 TEST_F(PartitionCowCreatorTest, IntersectSelf) {
@@ -223,6 +229,8 @@
 }
 
 TEST(DmSnapshotInternals, CowSizeCalculator) {
+    SKIP_IF_NON_VIRTUAL_AB();
+
     DmSnapCowSizeCalculator cc(512, 8);
     unsigned long int b;
 
@@ -286,7 +294,9 @@
     std::optional<InstallOperation> expected_output;
 };
 
-class OptimizeOperationTest : public ::testing::TestWithParam<OptimizeOperationTestParam> {};
+class OptimizeOperationTest : public ::testing::TestWithParam<OptimizeOperationTestParam> {
+    void SetUp() override { SKIP_IF_NON_VIRTUAL_AB(); }
+};
 TEST_P(OptimizeOperationTest, Test) {
     InstallOperation actual_output;
     EXPECT_EQ(GetParam().expected_output.has_value(),
diff --git a/fs_mgr/libsnapshot/snapshot_metadata_updater_test.cpp b/fs_mgr/libsnapshot/snapshot_metadata_updater_test.cpp
index 5530e59..0a16c03 100644
--- a/fs_mgr/libsnapshot/snapshot_metadata_updater_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_metadata_updater_test.cpp
@@ -43,11 +43,11 @@
 
 class SnapshotMetadataUpdaterTest : public ::testing::TestWithParam<uint32_t> {
   public:
-    SnapshotMetadataUpdaterTest() {
-        is_virtual_ab_ = android::base::GetBoolProperty("ro.virtual_ab.enabled", false);
-    }
+    SnapshotMetadataUpdaterTest() = default;
 
     void SetUp() override {
+        SKIP_IF_NON_VIRTUAL_AB();
+
         target_slot_ = GetParam();
         target_suffix_ = SlotSuffixForSlotNumber(target_slot_);
         SnapshotTestPropertyFetcher::SetUp(SlotSuffixForSlotNumber(1 - target_slot_));
@@ -68,7 +68,11 @@
         ASSERT_TRUE(FillFakeMetadata(builder_.get(), manifest_, target_suffix_));
     }
 
-    void TearDown() override { SnapshotTestPropertyFetcher::TearDown(); }
+    void TearDown() override {
+        RETURN_IF_NON_VIRTUAL_AB();
+
+        SnapshotTestPropertyFetcher::TearDown();
+    }
 
     // Append suffix to name.
     std::string T(std::string_view name) { return std::string(name) + target_suffix_; }
@@ -127,7 +131,6 @@
                                   << ".";
     }
 
-    bool is_virtual_ab_;
     std::unique_ptr<MetadataBuilder> builder_;
     uint32_t target_slot_;
     std::string target_suffix_;
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index 2bd0135..6ff935b 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -83,9 +83,7 @@
 
 class SnapshotTest : public ::testing::Test {
   public:
-    SnapshotTest() : dm_(DeviceMapper::Instance()) {
-        is_virtual_ab_ = android::base::GetBoolProperty("ro.virtual_ab.enabled", false);
-    }
+    SnapshotTest() : dm_(DeviceMapper::Instance()) {}
 
     // This is exposed for main.
     void Cleanup() {
@@ -95,7 +93,7 @@
 
   protected:
     void SetUp() override {
-        if (!is_virtual_ab_) GTEST_SKIP() << "Test for Virtual A/B devices only";
+        SKIP_IF_NON_VIRTUAL_AB();
 
         SnapshotTestPropertyFetcher::SetUp();
         InitializeState();
@@ -106,7 +104,7 @@
     }
 
     void TearDown() override {
-        if (!is_virtual_ab_) return;
+        RETURN_IF_NON_VIRTUAL_AB();
 
         lock_ = nullptr;
 
@@ -341,7 +339,6 @@
     }
 
     static constexpr std::chrono::milliseconds snapshot_timeout_ = 5s;
-    bool is_virtual_ab_;
     DeviceMapper& dm_;
     std::unique_ptr<SnapshotManager::LockedFile> lock_;
     android::fiemap::IImageManager* image_manager_ = nullptr;
@@ -722,11 +719,13 @@
 class LockTest : public ::testing::Test {
   public:
     void SetUp() {
+        SKIP_IF_NON_VIRTUAL_AB();
         first_consumer.StartHandleRequestsInBackground();
         second_consumer.StartHandleRequestsInBackground();
     }
 
     void TearDown() {
+        RETURN_IF_NON_VIRTUAL_AB();
         EXPECT_TRUE(first_consumer.MakeRequest(Request::EXIT));
         EXPECT_TRUE(second_consumer.MakeRequest(Request::EXIT));
     }
@@ -770,7 +769,7 @@
 class SnapshotUpdateTest : public SnapshotTest {
   public:
     void SetUp() override {
-        if (!is_virtual_ab_) GTEST_SKIP() << "Test for Virtual A/B devices only";
+        SKIP_IF_NON_VIRTUAL_AB();
 
         SnapshotTest::SetUp();
         Cleanup();
@@ -832,7 +831,7 @@
         }
     }
     void TearDown() override {
-        if (!is_virtual_ab_) return;
+        RETURN_IF_NON_VIRTUAL_AB();
 
         Cleanup();
         SnapshotTest::TearDown();
@@ -1365,13 +1364,17 @@
     ASSERT_EQ(UpdateState::MergeCompleted, init->ProcessUpdateState());
 }
 
-class MetadataMountedTest : public SnapshotUpdateTest {
+class MetadataMountedTest : public ::testing::Test {
   public:
+    // This is so main() can instantiate this to invoke Cleanup.
+    virtual void TestBody() override {}
     void SetUp() override {
+        SKIP_IF_NON_VIRTUAL_AB();
         metadata_dir_ = test_device->GetMetadataDir();
         ASSERT_TRUE(ReadDefaultFstab(&fstab_));
     }
     void TearDown() override {
+        RETURN_IF_NON_VIRTUAL_AB();
         SetUp();
         // Remount /metadata
         test_device->set_recovery(false);
@@ -1702,8 +1705,6 @@
 };
 
 TEST_P(FlashAfterUpdateTest, FlashSlotAfterUpdate) {
-    if (!is_virtual_ab_) GTEST_SKIP() << "Test for Virtual A/B devices only";
-
     // OTA client blindly unmaps all partitions that are possibly mapped.
     for (const auto& name : {"sys_b", "vnd_b", "prd_b"}) {
         ASSERT_TRUE(sm->UnmapUpdateSnapshot(name));
@@ -1803,14 +1804,13 @@
 class ImageManagerTest : public SnapshotTest, public WithParamInterface<uint64_t> {
   protected:
     void SetUp() override {
-        if (!is_virtual_ab_) GTEST_SKIP() << "Test for Virtual A/B devices only";
-
+        SKIP_IF_NON_VIRTUAL_AB();
         SnapshotTest::SetUp();
         userdata_ = std::make_unique<LowSpaceUserdata>();
         ASSERT_TRUE(userdata_->Init(GetParam()));
     }
     void TearDown() override {
-        if (!is_virtual_ab_) return;
+        RETURN_IF_NON_VIRTUAL_AB();
         return;  // BUG(149738928)
 
         EXPECT_TRUE(!image_manager_->BackingImageExists(kImageName) ||
@@ -1852,11 +1852,6 @@
 
 INSTANTIATE_TEST_SUITE_P(ImageManagerTest, ImageManagerTest, ValuesIn(ImageManagerTestParams()));
 
-}  // namespace snapshot
-}  // namespace android
-
-using namespace android::snapshot;
-
 bool Mkdir(const std::string& path) {
     if (mkdir(path.c_str(), 0700) && errno != EEXIST) {
         std::cerr << "Could not mkdir " << path << ": " << strerror(errno) << std::endl;
@@ -1865,8 +1860,21 @@
     return true;
 }
 
-int main(int argc, char** argv) {
-    ::testing::InitGoogleTest(&argc, argv);
+class SnapshotTestEnvironment : public ::testing::Environment {
+  public:
+    ~SnapshotTestEnvironment() override {}
+    void SetUp() override;
+    void TearDown() override;
+
+  private:
+    std::unique_ptr<IImageManager> super_images_;
+};
+
+void SnapshotTestEnvironment::SetUp() {
+    // b/163082876: GTEST_SKIP in Environment will make atest report incorrect results. Until
+    // that is fixed, don't call GTEST_SKIP here, but instead call GTEST_SKIP in individual test
+    // suites.
+    RETURN_IF_NON_VIRTUAL_AB_MSG("Virtual A/B is not enabled, skipping global setup.\n");
 
     std::vector<std::string> paths = {
             // clang-format off
@@ -1879,18 +1887,13 @@
             // clang-format on
     };
     for (const auto& path : paths) {
-        if (!Mkdir(path)) {
-            return 1;
-        }
+        ASSERT_TRUE(Mkdir(path));
     }
 
     // Create this once, otherwise, gsid will start/stop between each test.
     test_device = new TestDeviceInfo();
     sm = SnapshotManager::New(test_device);
-    if (!sm) {
-        std::cerr << "Could not create snapshot manager\n";
-        return 1;
-    }
+    ASSERT_NE(nullptr, sm) << "Could not create snapshot manager";
 
     // Clean up previous run.
     MetadataMountedTest().TearDown();
@@ -1898,31 +1901,35 @@
     SnapshotTest().Cleanup();
 
     // Use a separate image manager for our fake super partition.
-    auto super_images = IImageManager::Open("ota/test/super", 10s);
-    if (!super_images) {
-        std::cerr << "Could not create image manager\n";
-        return 1;
-    }
+    super_images_ = IImageManager::Open("ota/test/super", 10s);
+    ASSERT_NE(nullptr, super_images_) << "Could not create image manager";
 
     // Clean up any old copy.
-    DeleteBackingImage(super_images.get(), "fake-super");
+    DeleteBackingImage(super_images_.get(), "fake-super");
 
     // Create and map the fake super partition.
     static constexpr int kImageFlags =
             IImageManager::CREATE_IMAGE_DEFAULT | IImageManager::CREATE_IMAGE_ZERO_FILL;
-    if (!super_images->CreateBackingImage("fake-super", kSuperSize, kImageFlags)) {
-        std::cerr << "Could not create fake super partition\n";
-        return 1;
-    }
-    if (!super_images->MapImageDevice("fake-super", 10s, &fake_super)) {
-        std::cerr << "Could not map fake super partition\n";
-        return 1;
-    }
+    ASSERT_TRUE(super_images_->CreateBackingImage("fake-super", kSuperSize, kImageFlags))
+            << "Could not create fake super partition";
+
+    ASSERT_TRUE(super_images_->MapImageDevice("fake-super", 10s, &fake_super))
+            << "Could not map fake super partition";
     test_device->set_fake_super(fake_super);
+}
 
-    auto result = RUN_ALL_TESTS();
+void SnapshotTestEnvironment::TearDown() {
+    RETURN_IF_NON_VIRTUAL_AB();
+    if (super_images_ != nullptr) {
+        DeleteBackingImage(super_images_.get(), "fake-super");
+    }
+}
 
-    DeleteBackingImage(super_images.get(), "fake-super");
+}  // namespace snapshot
+}  // namespace android
 
-    return result;
+int main(int argc, char** argv) {
+    ::testing::InitGoogleTest(&argc, argv);
+    ::testing::AddGlobalTestEnvironment(new ::android::snapshot::SnapshotTestEnvironment());
+    return RUN_ALL_TESTS();
 }
diff --git a/fs_mgr/libsnapshot/test_helpers.cpp b/fs_mgr/libsnapshot/test_helpers.cpp
index de3d912..b07bf91 100644
--- a/fs_mgr/libsnapshot/test_helpers.cpp
+++ b/fs_mgr/libsnapshot/test_helpers.cpp
@@ -18,6 +18,7 @@
 
 #include <android-base/file.h>
 #include <android-base/logging.h>
+#include <android-base/properties.h>
 #include <android-base/strings.h>
 #include <android-base/unique_fd.h>
 #include <gtest/gtest.h>
@@ -241,5 +242,9 @@
     return bsize_;
 }
 
+bool IsVirtualAbEnabled() {
+    return android::base::GetBoolProperty("ro.virtual_ab.enabled", false);
+}
+
 }  // namespace snapshot
 }  // namespace android
diff --git a/logd/CommandListener.cpp b/logd/CommandListener.cpp
index f2fe7ef..0ba1621 100644
--- a/logd/CommandListener.cpp
+++ b/logd/CommandListener.cpp
@@ -32,6 +32,7 @@
 #include <string>
 
 #include <android-base/logging.h>
+#include <android-base/parseint.h>
 #include <android-base/stringprintf.h>
 #include <cutils/sockets.h>
 #include <log/log_properties.h>
@@ -64,53 +65,58 @@
     }
 }
 
-int CommandListener::ClearCmd::runCommand(SocketClient* cli, int argc,
-                                          char** argv) {
+template <typename F>
+static int LogIdCommand(SocketClient* cli, int argc, char** argv, F&& function) {
     setname();
+    if (argc < 2) {
+        cli->sendMsg("Missing Argument");
+        return 0;
+    }
+
+    int log_id;
+    if (!android::base::ParseInt(argv[1], &log_id, static_cast<int>(LOG_ID_MAIN),
+                                 static_cast<int>(LOG_ID_KERNEL))) {
+        cli->sendMsg("Range Error");
+        return 0;
+    }
+
+    function(static_cast<log_id_t>(log_id));
+    return 0;
+}
+
+int CommandListener::ClearCmd::runCommand(SocketClient* cli, int argc, char** argv) {
     uid_t uid = cli->getUid();
     if (clientHasLogCredentials(cli)) {
         uid = AID_ROOT;
     }
 
-    if (argc < 2) {
-        cli->sendMsg("Missing Argument");
-        return 0;
-    }
-
-    int id = atoi(argv[1]);
-    if ((id < LOG_ID_MIN) || (LOG_ID_MAX <= id)) {
-        cli->sendMsg("Range Error");
-        return 0;
-    }
-
-    cli->sendMsg(buf()->Clear((log_id_t)id, uid) ? "success" : "busy");
-    return 0;
+    return LogIdCommand(cli, argc, argv, [&](log_id_t id) {
+        cli->sendMsg(buf()->Clear(id, uid) ? "success" : "busy");
+    });
 }
 
-int CommandListener::GetBufSizeCmd::runCommand(SocketClient* cli, int argc,
-                                               char** argv) {
-    setname();
-    if (argc < 2) {
-        cli->sendMsg("Missing Argument");
-        return 0;
-    }
+template <typename F>
+static int LogSizeCommand(SocketClient* cli, int argc, char** argv, F&& size_function) {
+    return LogIdCommand(cli, argc, argv, [&](log_id_t log_id) {
+        cli->sendMsg(std::to_string(size_function(log_id)).c_str());
+    });
+}
 
-    int id = atoi(argv[1]);
-    if ((id < LOG_ID_MIN) || (LOG_ID_MAX <= id)) {
-        cli->sendMsg("Range Error");
-        return 0;
-    }
+int CommandListener::GetBufSizeCmd::runCommand(SocketClient* cli, int argc, char** argv) {
+    return LogSizeCommand(cli, argc, argv, [this](log_id_t id) { return buf()->GetSize(id); });
+}
 
-    size_t size = buf()->GetSize(static_cast<log_id_t>(id));
-    char buf[512];
-    snprintf(buf, sizeof(buf), "%zu", size);
-    cli->sendMsg(buf);
-    return 0;
+int CommandListener::GetBufSizeReadableCmd::runCommand(SocketClient* cli, int argc, char** argv) {
+    return LogSizeCommand(cli, argc, argv,
+                          [this](log_id_t id) { return stats()->SizeReadable(id); });
+}
+
+int CommandListener::GetBufSizeUsedCmd::runCommand(SocketClient* cli, int argc, char** argv) {
+    return LogSizeCommand(cli, argc, argv, [this](log_id_t id) { return stats()->Sizes(id); });
 }
 
 int CommandListener::SetBufSizeCmd::runCommand(SocketClient* cli, int argc,
                                                char** argv) {
-    setname();
     if (!clientHasLogCredentials(cli)) {
         cli->sendMsg("Permission Denied");
         return 0;
@@ -120,62 +126,11 @@
         cli->sendMsg("Missing Argument");
         return 0;
     }
-
-    int id = atoi(argv[1]);
-    if ((id < LOG_ID_MIN) || (LOG_ID_MAX <= id)) {
-        cli->sendMsg("Range Error");
-        return 0;
-    }
-
     size_t size = atol(argv[2]);
-    if (!buf()->SetSize(static_cast<log_id_t>(id), size)) {
-        cli->sendMsg("Range Error");
-        return 0;
-    }
 
-    cli->sendMsg("success");
-    return 0;
-}
-
-int CommandListener::GetBufSizeReadableCmd::runCommand(SocketClient* cli, int argc, char** argv) {
-    setname();
-    if (argc < 2) {
-        cli->sendMsg("Missing Argument");
-        return 0;
-    }
-
-    int id = atoi(argv[1]);
-    if (id < LOG_ID_MIN || LOG_ID_MAX <= id) {
-        cli->sendMsg("Range Error");
-        return 0;
-    }
-
-    size_t size = stats()->SizeReadable(static_cast<log_id_t>(id));
-    char buf[512];
-    snprintf(buf, sizeof(buf), "%zu", size);
-    cli->sendMsg(buf);
-    return 0;
-}
-
-int CommandListener::GetBufSizeUsedCmd::runCommand(SocketClient* cli, int argc,
-                                                   char** argv) {
-    setname();
-    if (argc < 2) {
-        cli->sendMsg("Missing Argument");
-        return 0;
-    }
-
-    int id = atoi(argv[1]);
-    if ((id < LOG_ID_MIN) || (LOG_ID_MAX <= id)) {
-        cli->sendMsg("Range Error");
-        return 0;
-    }
-
-    size_t size = stats()->Sizes(static_cast<log_id_t>(id));
-    char buf[512];
-    snprintf(buf, sizeof(buf), "%zu", size);
-    cli->sendMsg(buf);
-    return 0;
+    return LogIdCommand(cli, argc, argv, [&](log_id_t log_id) {
+        cli->sendMsg(buf()->SetSize(log_id, size) ? "success" : "busy");
+    });
 }
 
 // This returns a string with a length prefix with the format <length>\n<data>\n\f.  The length
@@ -200,8 +155,7 @@
     return android::base::StringPrintf("%zu\n%s\n\f", total_size, str.c_str());
 }
 
-int CommandListener::GetStatisticsCmd::runCommand(SocketClient* cli, int argc,
-                                                  char** argv) {
+int CommandListener::GetStatisticsCmd::runCommand(SocketClient* cli, int argc, char** argv) {
     setname();
     uid_t uid = cli->getUid();
     if (clientHasLogCredentials(cli)) {
@@ -266,8 +220,7 @@
     return 0;
 }
 
-int CommandListener::GetEventTagCmd::runCommand(SocketClient* cli, int argc,
-                                                char** argv) {
+int CommandListener::GetEventTagCmd::runCommand(SocketClient* cli, int argc, char** argv) {
     setname();
     uid_t uid = cli->getUid();
     if (clientHasLogCredentials(cli)) {
@@ -311,8 +264,7 @@
     return 0;
 }
 
-int CommandListener::ReinitCmd::runCommand(SocketClient* cli, int /*argc*/,
-                                           char** /*argv*/) {
+int CommandListener::ReinitCmd::runCommand(SocketClient* cli, int, char**) {
     setname();
 
     LOG(INFO) << "logd reinit";
@@ -335,8 +287,7 @@
     return 0;
 }
 
-int CommandListener::ExitCmd::runCommand(SocketClient* cli, int /*argc*/,
-                                         char** /*argv*/) {
+int CommandListener::ExitCmd::runCommand(SocketClient* cli, int, char**) {
     setname();
 
     cli->sendMsg("success");