lshal: remove ListCommand addLine

Remove obnoxious addLine(...) in ListCommand.cpp by moving
the feature of selecting columns into the "Table" class.

Test: lshal
Test: lshal -m
Test: lshal -d (shows debug info for context hub)
Test: lshal_test
Bug: 35389839
Change-Id: Ieb4a6e544ef39c9f1a63b046a44b6a8e1416ea62

7 files changed, 266 insertions, 134 deletions

diff --git a/cmds/lshal/Android.bp b/cmds/lshal/Android.bp
index 2eed1cebc2..fab7bfd0e8 100644
--- a/cmds/lshal/Android.bp
+++ b/cmds/lshal/Android.bp
@@ -28,6 +28,7 @@ cc_library_shared {
         "Lshal.cpp",
         "ListCommand.cpp",
         "PipeRelay.cpp",
+        "TableEntry.cpp",
         "TextTable.cpp",
         "utils.cpp",
     ],
diff --git a/cmds/lshal/ListCommand.cpp b/cmds/lshal/ListCommand.cpp
index 31c42e7776..6ee162b017 100644
--- a/cmds/lshal/ListCommand.cpp
+++ b/cmds/lshal/ListCommand.cpp
@@ -35,7 +35,6 @@
 
 #include "Lshal.h"
 #include "PipeRelay.h"
-#include "TextTable.h"
 #include "Timeout.h"
 #include "utils.h"
 
@@ -207,48 +206,6 @@ void ListCommand::postprocess() {
     }
 }
 
-void ListCommand::addLine(TextTable *textTable, const std::string &interfaceName,
-                          const std::string &transport, const std::string &arch,
-                          const std::string &threadUsage, const std::string &server,
-                          const std::string &serverCmdline, const std::string &address,
-                          const std::string &clients, const std::string &clientCmdlines) const {
-    std::vector<std::string> columns;
-    for (TableColumnType type : mSelectedColumns) {
-        switch (type) {
-            case TableColumnType::INTERFACE_NAME: {
-                columns.push_back(interfaceName);
-            } break;
-            case TableColumnType::TRANSPORT: {
-                columns.push_back(transport);
-            } break;
-            case TableColumnType::ARCH: {
-                columns.push_back(arch);
-            } break;
-            case TableColumnType::THREADS: {
-                columns.push_back(threadUsage);
-            } break;
-            case TableColumnType::SERVER_ADDR: {
-                columns.push_back(address);
-            } break;
-            case TableColumnType::SERVER_PID: {
-                if (mEnableCmdlines) {
-                    columns.push_back(serverCmdline);
-                } else {
-                    columns.push_back(server);
-                }
-            } break;
-            case TableColumnType::CLIENT_PIDS: {
-                if (mEnableCmdlines) {
-                    columns.push_back(clientCmdlines);
-                } else {
-                    columns.push_back(clients);
-                }
-            } break;
-        }
-    }
-    textTable->add(std::move(columns));
-}
-
 static inline bool findAndBumpVersion(vintf::ManifestHal* hal, const vintf::Version& version) {
     for (vintf::Version& v : hal->versions) {
         if (v.majorVer == version.majorVer) {
@@ -372,24 +329,6 @@ void ListCommand::dumpVintf() const {
     mOut << vintf::gHalManifestConverter(manifest);
 }
 
-static const std::string &getArchString(Architecture arch) {
-    static const std::string sStr64 = "64";
-    static const std::string sStr32 = "32";
-    static const std::string sStrBoth = "32+64";
-    static const std::string sStrUnknown = "";
-    switch (arch) {
-        case ARCH64:
-            return sStr64;
-        case ARCH32:
-            return sStr32;
-        case ARCH_BOTH:
-            return sStrBoth;
-        case ARCH_UNKNOWN: // fall through
-        default:
-            return sStrUnknown;
-    }
-}
-
 static Architecture fromBaseArchitecture(::android::hidl::base::V1_0::DebugInfo::Architecture a) {
     switch (a) {
         case ::android::hidl::base::V1_0::DebugInfo::Architecture::IS_64BIT:
@@ -402,63 +341,43 @@ static Architecture fromBaseArchitecture(::android::hidl::base::V1_0::DebugInfo:
     }
 }
 
-void ListCommand::addLine(TextTable *table, const TableEntry &entry) {
-    addLine(table, entry.interfaceName, entry.transport, getArchString(entry.arch),
-            entry.getThreadUsage(),
-            entry.serverPid == NO_PID ? "N/A" : std::to_string(entry.serverPid),
-            entry.serverCmdline,
-            entry.serverObjectAddress == NO_PTR ? "N/A" : toHexString(entry.serverObjectAddress),
-            join(entry.clientPids, " "), join(entry.clientCmdlines, ";"));
-}
-
 void ListCommand::dumpTable() {
     if (mNeat) {
-        TextTable textTable;
-        forEachTable([this, &textTable](const Table &table) {
-            for (const auto &entry : table) addLine(&textTable, entry);
-        });
-        textTable.dump(mOut.buf());
+        MergedTable({&mServicesTable, &mPassthroughRefTable, &mImplementationsTable})
+            .createTextTable().dump(mOut.buf());
         return;
     }
 
-    mServicesTable.description =
-            "All binderized services (registered services through hwservicemanager)";
-    mPassthroughRefTable.description =
+    mServicesTable.setDescription(
+            "All binderized services (registered services through hwservicemanager)");
+    mPassthroughRefTable.setDescription(
             "All interfaces that getService() has ever return as a passthrough interface;\n"
             "PIDs / processes shown below might be inaccurate because the process\n"
             "might have relinquished the interface or might have died.\n"
             "The Server / Server CMD column can be ignored.\n"
             "The Clients / Clients CMD column shows all process that have ever dlopen'ed \n"
-            "the library and successfully fetched the passthrough implementation.";
-    mImplementationsTable.description =
-            "All available passthrough implementations (all -impl.so files)";
+            "the library and successfully fetched the passthrough implementation.");
+    mImplementationsTable.setDescription(
+            "All available passthrough implementations (all -impl.so files)");
 
     forEachTable([this](const Table &table) {
-        TextTable textTable;
-
-        textTable.add(table.description);
-        addLine(&textTable, "Interface", "Transport", "Arch", "Thread Use", "Server", "Server CMD",
-                "PTR", "Clients", "Clients CMD");
-
-        for (const auto &entry : table) {
-            addLine(&textTable, entry);
-            // We're only interested in dumping debug info for already
-            // instantiated services. There's little value in dumping the
-            // debug info for a service we create on the fly, so we only operate
-            // on the "mServicesTable".
-            if (mEmitDebugInfo && &table == &mServicesTable) {
+
+        // We're only interested in dumping debug info for already
+        // instantiated services. There's little value in dumping the
+        // debug info for a service we create on the fly, so we only operate
+        // on the "mServicesTable".
+        std::function<std::string(const std::string&)> emitDebugInfo = nullptr;
+        if (mEmitDebugInfo && &table == &mServicesTable) {
+            emitDebugInfo = [this](const auto& iName) {
                 std::stringstream out;
-                auto pair = splitFirst(entry.interfaceName, '/');
+                auto pair = splitFirst(iName, '/');
                 mLshal.emitDebugInfo(pair.first, pair.second, {}, out,
                                      NullableOStream<std::ostream>(nullptr));
-                textTable.add(out.str());
-            }
+                return out.str();
+            };
         }
-
-        // Add empty line after each table
-        textTable.add();
-
-        textTable.dump(mOut.buf());
+        table.createTextTable(mNeat, emitDebugInfo).dump(mOut.buf());
+        mOut << std::endl;
     });
 }
 
@@ -489,7 +408,7 @@ void ListCommand::putEntry(TableEntrySource source, TableEntry &&entry) {
             mErr << "Error: Unknown source of entry " << source << std::endl;
     }
     if (table) {
-        table->entries.push_back(std::forward<TableEntry>(entry));
+        table->add(std::forward<TableEntry>(entry));
     }
 }
 
@@ -693,6 +612,9 @@ Status ListCommand::parseArgs(const std::string &command, const Arg &arg) {
         { 0,          0,                 0,  0  }
     };
 
+    std::vector<TableColumnType> selectedColumns;
+    bool enableCmdlines = false;
+
     int optionIndex;
     int c;
     // Lshal::parseArgs has set optind to the next option to parse
@@ -728,35 +650,35 @@ Status ListCommand::parseArgs(const std::string &command, const Arg &arg) {
             mVintf = true;
         }
         case 'i': {
-            mSelectedColumns.push_back(TableColumnType::INTERFACE_NAME);
+            selectedColumns.push_back(TableColumnType::INTERFACE_NAME);
             break;
         }
         case 't': {
-            mSelectedColumns.push_back(TableColumnType::TRANSPORT);
+            selectedColumns.push_back(TableColumnType::TRANSPORT);
             break;
         }
         case 'r': {
-            mSelectedColumns.push_back(TableColumnType::ARCH);
+            selectedColumns.push_back(TableColumnType::ARCH);
             break;
         }
         case 'p': {
-            mSelectedColumns.push_back(TableColumnType::SERVER_PID);
+            selectedColumns.push_back(TableColumnType::SERVER_PID);
             break;
         }
         case 'a': {
-            mSelectedColumns.push_back(TableColumnType::SERVER_ADDR);
+            selectedColumns.push_back(TableColumnType::SERVER_ADDR);
             break;
         }
         case 'c': {
-            mSelectedColumns.push_back(TableColumnType::CLIENT_PIDS);
+            selectedColumns.push_back(TableColumnType::CLIENT_PIDS);
             break;
         }
         case 'e': {
-            mSelectedColumns.push_back(TableColumnType::THREADS);
+            selectedColumns.push_back(TableColumnType::THREADS);
             break;
         }
         case 'm': {
-            mEnableCmdlines = true;
+            enableCmdlines = true;
             break;
         }
         case 'd': {
@@ -796,10 +718,26 @@ Status ListCommand::parseArgs(const std::string &command, const Arg &arg) {
         return USAGE;
     }
 
-    if (mSelectedColumns.empty()) {
-        mSelectedColumns = {TableColumnType::INTERFACE_NAME, TableColumnType::THREADS,
+    if (selectedColumns.empty()) {
+        selectedColumns = {TableColumnType::INTERFACE_NAME, TableColumnType::THREADS,
                             TableColumnType::SERVER_PID, TableColumnType::CLIENT_PIDS};
     }
+
+    if (enableCmdlines) {
+        for (size_t i = 0; i < selectedColumns.size(); ++i) {
+            if (selectedColumns[i] == TableColumnType::SERVER_PID) {
+                selectedColumns[i] = TableColumnType::SERVER_CMD;
+            }
+            if (selectedColumns[i] == TableColumnType::CLIENT_PIDS) {
+                selectedColumns[i] = TableColumnType::CLIENT_CMDS;
+            }
+        }
+    }
+
+    forEachTable([&selectedColumns] (Table& table) {
+        table.setSelectedColumns(selectedColumns);
+    });
+
     return OK;
 }
 
diff --git a/cmds/lshal/ListCommand.h b/cmds/lshal/ListCommand.h
index 176d5b9a0a..d9d56a329a 100644
--- a/cmds/lshal/ListCommand.h
+++ b/cmds/lshal/ListCommand.h
@@ -82,11 +82,7 @@ private:
     NullableOStream<std::ostream> mOut;
     NullableOStream<std::ofstream> mFileOutput = nullptr;
     TableEntryCompare mSortColumn = nullptr;
-    std::vector<TableColumnType> mSelectedColumns;
-    // If true, cmdlines will be printed instead of pid.
-    bool mEnableCmdlines = false;
 
-    // If true, calls IBase::debug(...) on each service.
     bool mEmitDebugInfo = false;
 
     // If true, output in VINTF format.
diff --git a/cmds/lshal/TableEntry.cpp b/cmds/lshal/TableEntry.cpp
new file mode 100644
index 0000000000..dc6ccaeda5
--- /dev/null
+++ b/cmds/lshal/TableEntry.cpp
@@ -0,0 +1,156 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#define LOG_TAG "lshal"
+#include <android-base/logging.h>
+
+#include "TableEntry.h"
+
+#include "TextTable.h"
+#include "utils.h"
+
+namespace android {
+namespace lshal {
+
+static const std::string &getArchString(Architecture arch) {
+    static const std::string sStr64 = "64";
+    static const std::string sStr32 = "32";
+    static const std::string sStrBoth = "32+64";
+    static const std::string sStrUnknown = "";
+    switch (arch) {
+        case ARCH64:
+            return sStr64;
+        case ARCH32:
+            return sStr32;
+        case ARCH_BOTH:
+            return sStrBoth;
+        case ARCH_UNKNOWN: // fall through
+        default:
+            return sStrUnknown;
+    }
+}
+
+static std::string getTitle(TableColumnType type) {
+    switch (type) {
+        case TableColumnType::INTERFACE_NAME: {
+            return "Interface";
+        } break;
+        case TableColumnType::TRANSPORT: {
+            return "Transport";
+        } break;
+        case TableColumnType::SERVER_PID: {
+            return "Server";
+        } break;
+        case TableColumnType::SERVER_CMD: {
+            return "Server CMD";
+        }
+        case TableColumnType::SERVER_ADDR: {
+            return "PTR";
+        } break;
+        case TableColumnType::CLIENT_PIDS: {
+            return "Clients";
+        } break;
+        case TableColumnType::CLIENT_CMDS: {
+            return "Clients CMD";
+        } break;
+        case TableColumnType::ARCH: {
+            return "Arch";
+        } break;
+        case TableColumnType::THREADS: {
+            return "Thread Use";
+        } break;
+        default: {
+            LOG(FATAL) << "Should not reach here.";
+            return "";
+        }
+    }
+}
+
+std::string TableEntry::getField(TableColumnType type) const {
+    switch (type) {
+        case TableColumnType::INTERFACE_NAME: {
+            return interfaceName;
+        } break;
+        case TableColumnType::TRANSPORT: {
+            return transport;
+        } break;
+        case TableColumnType::SERVER_PID: {
+            return serverPid == NO_PID ? "N/A" : std::to_string(serverPid);
+        } break;
+        case TableColumnType::SERVER_CMD: {
+            return serverCmdline;
+        } break;
+        case TableColumnType::SERVER_ADDR: {
+            return serverObjectAddress == NO_PTR ? "N/A" : toHexString(serverObjectAddress);
+        } break;
+        case TableColumnType::CLIENT_PIDS: {
+            return join(clientPids, " ");
+        } break;
+        case TableColumnType::CLIENT_CMDS: {
+            return join(clientCmdlines, ";");
+        } break;
+        case TableColumnType::ARCH: {
+            return getArchString(arch);
+        } break;
+        case TableColumnType::THREADS: {
+            return getThreadUsage();
+        } break;
+        default: {
+            LOG(FATAL) << "Should not reach here.";
+            return "";
+        }
+    }
+}
+
+TextTable Table::createTextTable(bool neat,
+    const std::function<std::string(const std::string&)>& emitDebugInfo) const {
+
+    TextTable textTable;
+    std::vector<std::string> row;
+    if (!neat) {
+        textTable.add(mDescription);
+
+        row.clear();
+        for (TableColumnType type : mSelectedColumns) {
+            row.push_back(getTitle(type));
+        }
+        textTable.add(std::move(row));
+    }
+
+    for (const auto& entry : mEntries) {
+        row.clear();
+        for (TableColumnType type : mSelectedColumns) {
+            row.push_back(entry.getField(type));
+        }
+        textTable.add(std::move(row));
+
+        if (emitDebugInfo) {
+            std::string debugInfo = emitDebugInfo(entry.interfaceName);
+            if (!debugInfo.empty()) textTable.add(debugInfo);
+        }
+    }
+    return textTable;
+}
+
+TextTable MergedTable::createTextTable() {
+    TextTable textTable;
+    for (const Table* table : mTables) {
+        textTable.addAll(table->createTextTable());
+    }
+    return textTable;
+}
+
+} // namespace lshal
+} // namespace android
diff --git a/cmds/lshal/TableEntry.h b/cmds/lshal/TableEntry.h
index f18f38addf..dac8b5e3b7 100644
--- a/cmds/lshal/TableEntry.h
+++ b/cmds/lshal/TableEntry.h
@@ -23,6 +23,8 @@
 #include <vector>
 #include <iostream>
 
+#include "TextTable.h"
+
 namespace android {
 namespace lshal {
 
@@ -43,6 +45,18 @@ enum : unsigned int {
 };
 using Architecture = unsigned int;
 
+enum class TableColumnType : unsigned int {
+    INTERFACE_NAME,
+    TRANSPORT,
+    SERVER_PID,
+    SERVER_CMD,
+    SERVER_ADDR,
+    CLIENT_PIDS,
+    CLIENT_CMDS,
+    ARCH,
+    THREADS,
+};
+
 struct TableEntry {
     std::string interfaceName;
     std::string transport;
@@ -69,29 +83,46 @@ struct TableEntry {
 
         return std::to_string(threadUsage) + "/" + std::to_string(threadCount);
     }
+
+    std::string getField(TableColumnType type) const;
 };
 
-struct Table {
+using SelectedColumns = std::vector<TableColumnType>;
+
+class Table {
+public:
     using Entries = std::vector<TableEntry>;
-    std::string description;
-    Entries entries;
 
-    Entries::iterator begin() { return entries.begin(); }
-    Entries::const_iterator begin() const { return entries.begin(); }
-    Entries::iterator end() { return entries.end(); }
-    Entries::const_iterator end() const { return entries.end(); }
+    Entries::iterator begin() { return mEntries.begin(); }
+    Entries::const_iterator begin() const { return mEntries.begin(); }
+    Entries::iterator end() { return mEntries.end(); }
+    Entries::const_iterator end() const { return mEntries.end(); }
+
+    void add(TableEntry&& entry) { mEntries.push_back(std::move(entry)); }
+
+    void setSelectedColumns(const SelectedColumns& s) { mSelectedColumns = s; }
+    const SelectedColumns& getSelectedColumns() const { return mSelectedColumns; }
+
+    void setDescription(std::string&& d) { mDescription = std::move(d); }
+
+    // Write table content.
+    TextTable createTextTable(bool neat = true,
+        const std::function<std::string(const std::string&)>& emitDebugInfo = nullptr) const;
+
+private:
+    std::string mDescription;
+    Entries mEntries;
+    SelectedColumns mSelectedColumns;
 };
 
 using TableEntryCompare = std::function<bool(const TableEntry &, const TableEntry &)>;
 
-enum class TableColumnType : unsigned int {
-    INTERFACE_NAME,
-    TRANSPORT,
-    SERVER_PID,
-    SERVER_ADDR,
-    CLIENT_PIDS,
-    ARCH,
-    THREADS,
+class MergedTable {
+public:
+    MergedTable(std::vector<const Table*>&& tables) : mTables(std::move(tables)) {}
+    TextTable createTextTable();
+private:
+    std::vector<const Table*> mTables;
 };
 
 enum {
diff --git a/cmds/lshal/TextTable.cpp b/cmds/lshal/TextTable.cpp
index a35917c1e6..eca9061034 100644
--- a/cmds/lshal/TextTable.cpp
+++ b/cmds/lshal/TextTable.cpp
@@ -53,5 +53,15 @@ void TextTable::dump(std::ostream& out) const {
     }
 }
 
+void TextTable::addAll(TextTable&& other) {
+    for (auto&& row : other.mTable) {
+        if (row.isRow()) {
+            computeWidth(row.fields());
+        }
+
+        mTable.emplace_back(std::move(row));
+    }
+}
+
 } // namespace lshal
 } // namespace android
diff --git a/cmds/lshal/TextTable.h b/cmds/lshal/TextTable.h
index 4636f15a27..91d522aef7 100644
--- a/cmds/lshal/TextTable.h
+++ b/cmds/lshal/TextTable.h
@@ -21,8 +21,6 @@
 #include <string>
 #include <vector>
 
-#include "TableEntry.h"
-
 namespace android {
 namespace lshal {
 
@@ -68,6 +66,8 @@ public:
     void add(const std::string& s) { mTable.emplace_back(s); }
     void add(std::string&& s) { mTable.emplace_back(std::move(s)); }
 
+    void addAll(TextTable&& other);
+
     // Prints the table to out, with column widths adjusted appropriately according
     // to the content.
     void dump(std::ostream& out) const;