From b6181e3e46dee981f9476b16f7d01ff5c0bb3107 Mon Sep 17 00:00:00 2001
From: David Anderson <dvander@google.com>
Date: Tue, 19 Jun 2018 15:37:25 -0700
Subject: [PATCH] libdm: Clean up error messages and dm_ioctl use.

This change removes unnecessary malloc() calls for dm_ioctls. It also
simplifies and fixes line endings on some error messages.

Bug: 110035986
Test: N/A
Change-Id: I2f56e5dab7f25cd9b2f80896f80101db56228981
---
 fs_mgr/libdm/dm.cpp    | 36 +++++++++++++-----------------------
 fs_mgr/tools/dmctl.cpp |  8 ++++----
 2 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/fs_mgr/libdm/dm.cpp b/fs_mgr/libdm/dm.cpp
index 57c127081..e8bae60ef 100644
--- a/fs_mgr/libdm/dm.cpp
+++ b/fs_mgr/libdm/dm.cpp
@@ -51,23 +51,18 @@ bool DeviceMapper::CreateDevice(const std::string& name) {
         return false;
     }
 
-    std::unique_ptr<struct dm_ioctl, decltype(&free)> io(
-            static_cast<struct dm_ioctl*>(malloc(sizeof(struct dm_ioctl))), free);
-    if (io == nullptr) {
-        LOG(ERROR) << "Failed to allocate dm_ioctl";
-        return false;
-    }
-    InitIo(io.get(), name);
+    struct dm_ioctl io;
+    InitIo(&io, name);
 
-    if (ioctl(fd_, DM_DEV_CREATE, io.get())) {
-        PLOG(ERROR) << "DM_DEV_CREATE failed to create [" << name << "]";
+    if (ioctl(fd_, DM_DEV_CREATE, &io)) {
+        PLOG(ERROR) << "DM_DEV_CREATE failed for [" << name << "]";
         return false;
     }
 
     // Check to make sure the newly created device doesn't already have targets
     // added or opened by someone
-    CHECK(io->target_count == 0) << "Unexpected targets for newly created [" << name << "] device";
-    CHECK(io->open_count == 0) << "Unexpected opens for newly created [" << name << "] device";
+    CHECK(io.target_count == 0) << "Unexpected targets for newly created [" << name << "] device";
+    CHECK(io.open_count == 0) << "Unexpected opens for newly created [" << name << "] device";
 
     // Creates a new device mapper device with the name passed in
     return true;
@@ -84,22 +79,17 @@ bool DeviceMapper::DeleteDevice(const std::string& name) {
         return false;
     }
 
-    std::unique_ptr<struct dm_ioctl, decltype(&free)> io(
-            static_cast<struct dm_ioctl*>(malloc(sizeof(struct dm_ioctl))), free);
-    if (io == nullptr) {
-        LOG(ERROR) << "Failed to allocate dm_ioctl";
-        return false;
-    }
-    InitIo(io.get(), name);
+    struct dm_ioctl io;
+    InitIo(&io, name);
 
-    if (ioctl(fd_, DM_DEV_REMOVE, io.get())) {
-        PLOG(ERROR) << "DM_DEV_REMOVE failed to create [" << name << "]";
+    if (ioctl(fd_, DM_DEV_REMOVE, &io)) {
+        PLOG(ERROR) << "DM_DEV_REMOVE failed for [" << name << "]";
         return false;
     }
 
     // Check to make sure appropriate uevent is generated so ueventd will
     // do the right thing and remove the corresponding device node and symlinks.
-    CHECK(io->flags & DM_UEVENT_GENERATED_FLAG)
+    CHECK(io.flags & DM_UEVENT_GENERATED_FLAG)
             << "Didn't generate uevent for [" << name << "] removal";
 
     return true;
@@ -147,7 +137,7 @@ bool DeviceMapper::GetAvailableTargets(std::vector<DmTarget>* targets) {
     io->data_start = sizeof(*io);
 
     if (ioctl(fd_, DM_LIST_VERSIONS, io)) {
-        PLOG(ERROR) << "Failed to get DM_LIST_VERSIONS from kernel";
+        PLOG(ERROR) << "DM_LIST_VERSIONS failed";
         return false;
     }
 
@@ -209,7 +199,7 @@ bool DeviceMapper::GetAvailableDevices(std::vector<DmBlockDevice>* devices) {
     io->data_start = sizeof(*io);
 
     if (ioctl(fd_, DM_LIST_DEVICES, io)) {
-        PLOG(ERROR) << "Failed to get DM_LIST_DEVICES from kernel";
+        PLOG(ERROR) << "DM_LIST_DEVICES failed";
         return false;
     }
 
diff --git a/fs_mgr/tools/dmctl.cpp b/fs_mgr/tools/dmctl.cpp
index b40b83af6..e82c71872 100644
--- a/fs_mgr/tools/dmctl.cpp
+++ b/fs_mgr/tools/dmctl.cpp
@@ -49,14 +49,14 @@ static int Usage(void) {
 
 static int DmCreateCmdHandler(int argc, char** argv) {
     if (argc < 1) {
-        std::cerr << "DmCreateCmdHandler: atleast 'name' MUST be provided for target device";
+        std::cerr << "Usage: dmctl create <name> [table-args]" << std::endl;
         return -EINVAL;
     }
 
     std::string name = argv[0];
     DeviceMapper& dm = DeviceMapper::Instance();
     if (!dm.CreateDevice(name)) {
-        std::cerr << "DmCreateCmdHandler: Failed to create " << name << " device";
+        std::cerr << "Failed to create device-mapper device with name: " << name << std::endl;
         return -EIO;
     }
 
@@ -71,14 +71,14 @@ static int DmCreateCmdHandler(int argc, char** argv) {
 
 static int DmDeleteCmdHandler(int argc, char** argv) {
     if (argc < 1) {
-        std::cerr << "DmCreateCmdHandler: atleast 'name' MUST be provided for target device";
+        std::cerr << "Usage: dmctl delete <name>" << std::endl;
         return -EINVAL;
     }
 
     std::string name = argv[0];
     DeviceMapper& dm = DeviceMapper::Instance();
     if (!dm.DeleteDevice(name)) {
-        std::cerr << "DmCreateCmdHandler: Failed to create " << name << " device";
+        std::cerr << "Failed to delete [" << name << "]" << std::endl;
         return -EIO;
     }