Camera service: Fix dump() for treble path and rearrange it

Get camera service dump working again in the Treble path, and clean
up the formatting a bit

- Switch to dprintf instead of write() for most dump calls
- Add clearer headers for each section
- Add static metadata details to CameraProviderManager dump

Test: adb shell dumpsys media.camera with Treble both enabled and disabled
Bug: 32991422
Change-Id: Ie1d431b68649777bfe84fbb1be0687dd02e671af
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index d4bf23c..f429df8 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -2196,8 +2196,11 @@
         const std::set<userid_t>& newUserIds) {
     String8 newUsers = toString(newUserIds);
     String8 oldUsers = toString(oldUserIds);
+    if (oldUsers.size() == 0) {
+        oldUsers = "<None>";
+    }
     // Log the new and old users
-    logEvent(String8::format("USER_SWITCH previous allowed users: %s , current allowed users: %s",
+    logEvent(String8::format("USER_SWITCH previous allowed user IDs: %s, current allowed user IDs: %s",
             oldUsers.string(), newUsers.string()));
 }
 
@@ -2743,27 +2746,20 @@
 status_t CameraService::dump(int fd, const Vector<String16>& args) {
     ATRACE_CALL();
 
-    String8 result("Dump of the Camera Service:\n");
     if (checkCallingPermission(String16("android.permission.DUMP")) == false) {
-        result = result.format("Permission Denial: "
-                "can't dump CameraService from pid=%d, uid=%d\n",
+        dprintf(fd, "Permission Denial: can't dump CameraService from pid=%d, uid=%d\n",
                 getCallingPid(),
                 getCallingUid());
-        write(fd, result.string(), result.size());
         return NO_ERROR;
     }
-
     bool locked = tryLock(mServiceLock);
     // failed to lock - CameraService is probably deadlocked
     if (!locked) {
-        result.append("CameraService may be deadlocked\n");
-        write(fd, result.string(), result.size());
+        dprintf(fd, "!! CameraService may be deadlocked !!\n");
     }
 
-    bool hasClient = false;
     if (!mInitialized) {
-        result = String8::format("No camera HAL available!\n");
-        write(fd, result.string(), result.size());
+        dprintf(fd, "!! No camera HAL available !!\n");
 
         // Dump event log for error information
         dumpEventLog(fd);
@@ -2771,127 +2767,115 @@
         if (locked) mServiceLock.unlock();
         return NO_ERROR;
     }
-    if (mModule == nullptr) {
-        mCameraProviderManager->dump(fd, args);
-        // TODO - need way more dumping here
-
-        if (locked) mServiceLock.unlock();
-        return NO_ERROR;
-    }
-
-    result = String8::format("Camera module HAL API version: 0x%x\n", mModule->getHalApiVersion());
-    result.appendFormat("Camera module API version: 0x%x\n", mModule->getModuleApiVersion());
-    result.appendFormat("Camera module name: %s\n", mModule->getModuleName());
-    result.appendFormat("Camera module author: %s\n", mModule->getModuleAuthor());
-    result.appendFormat("Number of camera devices: %d\n", mNumberOfCameras);
-    result.appendFormat("Number of normal camera devices: %d\n", mNumberOfNormalCameras);
+    dprintf(fd, "\n== Service global info: ==\n\n");
+    dprintf(fd, "Number of camera devices: %d\n", mNumberOfCameras);
+    dprintf(fd, "Number of normal camera devices: %d\n", mNumberOfNormalCameras);
     String8 activeClientString = mActiveClientManager.toString();
-    result.appendFormat("Active Camera Clients:\n%s", activeClientString.string());
-    result.appendFormat("Allowed users:\n%s\n", toString(mAllowedUsers).string());
-
-    sp<VendorTagDescriptor> desc = VendorTagDescriptor::getGlobalVendorTagDescriptor();
-    if (desc == NULL) {
-        result.appendFormat("Vendor tags left unimplemented.\n");
-    } else {
-        result.appendFormat("Vendor tag definitions:\n");
-    }
-
-    write(fd, result.string(), result.size());
-
-    if (desc != NULL) {
-        desc->dump(fd, /*verbosity*/2, /*indentation*/4);
-    }
+    dprintf(fd, "Active Camera Clients:\n%s", activeClientString.string());
+    dprintf(fd, "Allowed user IDs: %s\n", toString(mAllowedUsers).string());
 
     dumpEventLog(fd);
 
     bool stateLocked = tryLock(mCameraStatesLock);
     if (!stateLocked) {
-        result = String8::format("CameraStates in use, may be deadlocked\n");
-        write(fd, result.string(), result.size());
+        dprintf(fd, "CameraStates in use, may be deadlocked\n");
     }
 
     for (auto& state : mCameraStates) {
         String8 cameraId = state.first;
-        result = String8::format("Camera %s information:\n", cameraId.string());
-        camera_info info;
 
-        status_t rc = mModule->getCameraInfo(cameraIdToInt(cameraId), &info);
-        if (rc != OK) {
-            result.appendFormat("  Error reading static information!\n");
-            write(fd, result.string(), result.size());
-        } else {
-            result.appendFormat("  Facing: %s\n",
-                    info.facing == CAMERA_FACING_BACK ? "BACK" :
-                    info.facing == CAMERA_FACING_FRONT ? "FRONT" : "EXTERNAL");
-            result.appendFormat("  Orientation: %d\n", info.orientation);
-            int deviceVersion;
-            if (mModule->getModuleApiVersion() < CAMERA_MODULE_API_VERSION_2_0) {
-                deviceVersion = CAMERA_DEVICE_API_VERSION_1_0;
-            } else {
-                deviceVersion = info.device_version;
-            }
+        dprintf(fd, "== Camera device %s dynamic info: ==\n", cameraId.string());
 
-            auto conflicting = state.second->getConflicting();
-            result.appendFormat("  Resource Cost: %d\n", state.second->getCost());
-            result.appendFormat("  Conflicting Devices:");
-            for (auto& id : conflicting) {
-                result.appendFormat(" %s", id.string());
-            }
-            if (conflicting.size() == 0) {
-                result.appendFormat(" NONE");
-            }
-            result.appendFormat("\n");
-
-            result.appendFormat("  Device version: %#x\n", deviceVersion);
-            if (deviceVersion >= CAMERA_DEVICE_API_VERSION_3_0) {
-                result.appendFormat("  Device static metadata:\n");
-                write(fd, result.string(), result.size());
-                dump_indented_camera_metadata(info.static_camera_characteristics,
-                        fd, /*verbosity*/2, /*indentation*/4);
-            } else {
-                write(fd, result.string(), result.size());
-                }
-
-            CameraParameters p = state.second->getShimParams();
-            if (!p.isEmpty()) {
-                result = String8::format("  Camera1 API shim is using parameters:\n        ");
-                write(fd, result.string(), result.size());
-                p.dump(fd, args);
-            }
+        CameraParameters p = state.second->getShimParams();
+        if (!p.isEmpty()) {
+            dprintf(fd, "  Camera1 API shim is using parameters:\n        ");
+            p.dump(fd, args);
         }
 
         auto clientDescriptor = mActiveClientManager.get(cameraId);
         if (clientDescriptor == nullptr) {
-            result = String8::format("  Device %s is closed, no client instance\n",
+            dprintf(fd, "  Device %s is closed, no client instance\n",
                     cameraId.string());
-            write(fd, result.string(), result.size());
             continue;
         }
-        hasClient = true;
-        result = String8::format("  Device %s is open. Client instance dump:\n\n",
+        dprintf(fd, "  Device %s is open. Client instance dump:\n",
                 cameraId.string());
-        result.appendFormat("Client priority level: %d\n", clientDescriptor->getPriority());
-        result.appendFormat("Client PID: %d\n", clientDescriptor->getOwnerId());
+        dprintf(fd, "    Client priority level: %d\n", clientDescriptor->getPriority());
+        dprintf(fd, "    Client PID: %d\n", clientDescriptor->getOwnerId());
 
         auto client = clientDescriptor->getValue();
-        result.appendFormat("Client package: %s\n",
+        dprintf(fd, "    Client package: %s\n",
                 String8(client->getPackageName()).string());
-        write(fd, result.string(), result.size());
 
         client->dumpClient(fd, args);
+
+        if (mModule != nullptr) {
+            dprintf(fd, "== Camera HAL device %s static information: ==\n", cameraId.string());
+
+            camera_info info;
+            status_t rc = mModule->getCameraInfo(cameraIdToInt(cameraId), &info);
+            int deviceVersion = -1;
+            if (rc != OK) {
+                dprintf(fd, "  Error reading static information!\n");
+            } else {
+                dprintf(fd, "  Facing: %s\n",
+                        info.facing == CAMERA_FACING_BACK ? "BACK" :
+                        info.facing == CAMERA_FACING_FRONT ? "FRONT" : "EXTERNAL");
+                dprintf(fd, "  Orientation: %d\n", info.orientation);
+
+                if (mModule->getModuleApiVersion() < CAMERA_MODULE_API_VERSION_2_0) {
+                    deviceVersion = CAMERA_DEVICE_API_VERSION_1_0;
+                } else {
+                    deviceVersion = info.device_version;
+                }
+            }
+
+            auto conflicting = state.second->getConflicting();
+            dprintf(fd, "  Resource Cost: %d\n", state.second->getCost());
+            dprintf(fd, "  Conflicting Devices:");
+            for (auto& id : conflicting) {
+                dprintf(fd, " %s", id.string());
+            }
+            if (conflicting.size() == 0) {
+                dprintf(fd, " NONE");
+            }
+            dprintf(fd, "\n");
+
+            dprintf(fd, "  Device version: %#x\n", deviceVersion);
+            if (deviceVersion >= CAMERA_DEVICE_API_VERSION_3_0) {
+                dprintf(fd, "  Device static metadata:\n");
+                dump_indented_camera_metadata(info.static_camera_characteristics,
+                        fd, /*verbosity*/2, /*indentation*/4);
+            }
+        }
+
     }
 
     if (stateLocked) mCameraStatesLock.unlock();
 
-    if (!hasClient) {
-        result = String8::format("\nNo active camera clients yet.\n");
-        write(fd, result.string(), result.size());
-    }
-
     if (locked) mServiceLock.unlock();
 
+    if (mModule == nullptr) {
+        mCameraProviderManager->dump(fd, args);
+    } else {
+        dprintf(fd, "\n== Camera Module HAL static info: ==\n");
+        dprintf(fd, "Camera module HAL API version: 0x%x\n", mModule->getHalApiVersion());
+        dprintf(fd, "Camera module API version: 0x%x\n", mModule->getModuleApiVersion());
+        dprintf(fd, "Camera module name: %s\n", mModule->getModuleName());
+        dprintf(fd, "Camera module author: %s\n", mModule->getModuleAuthor());
+    }
+
+    dprintf(fd, "\n== Vendor tags: ==\n\n");
+
+    sp<VendorTagDescriptor> desc = VendorTagDescriptor::getGlobalVendorTagDescriptor();
+    if (desc == NULL) {
+        dprintf(fd, "No vendor tags.\n");
+    } else {
+        desc->dump(fd, /*verbosity*/2, /*indentation*/2);
+    }
+
     // Dump camera traces if there were any
-    write(fd, "\n", 1);
+    dprintf(fd, "\n");
     camera3::CameraTraces::dump(fd, args);
 
     // Process dump arguments, if any
@@ -2904,19 +2888,18 @@
             if (i + 1 >= n) continue;
             String8 levelStr(args[i+1]);
             int level = atoi(levelStr.string());
-            result = String8::format("\nSetting log level to %d.\n", level);
+            dprintf(fd, "\nSetting log level to %d.\n", level);
             setLogLevel(level);
-            write(fd, result.string(), result.size());
         } else if (args[i] == unreachableOption) {
             // Dump memory analysis
             // TODO - should limit be an argument parameter?
             UnreachableMemoryInfo info;
             bool success = GetUnreachableMemory(info, /*limit*/ 10000);
             if (!success) {
-                dprintf(fd, "\nUnable to dump unreachable memory. "
-                        "Try disabling SELinux enforcement.\n");
+                dprintf(fd, "\n== Unable to dump unreachable memory. "
+                        "Try disabling SELinux enforcement. ==\n");
             } else {
-                dprintf(fd, "\nDumping unreachable memory:\n");
+                dprintf(fd, "\n== Dumping unreachable memory: ==\n");
                 std::string s = info.ToString(/*log_contents*/ true);
                 write(fd, s.c_str(), s.size());
             }
@@ -2926,21 +2909,19 @@
 }
 
 void CameraService::dumpEventLog(int fd) {
-    String8 result = String8("\nPrior client events (most recent at top):\n");
+    dprintf(fd, "\n== Camera service events log (most recent at top): ==\n");
 
     Mutex::Autolock l(mLogLock);
     for (const auto& msg : mEventLog) {
-        result.appendFormat("  %s\n", msg.string());
+        dprintf(fd, "  %s\n", msg.string());
     }
 
     if (mEventLog.size() == DEFAULT_EVENT_LOG_LENGTH) {
-        result.append("  ...\n");
+        dprintf(fd, "  ...\n");
     } else if (mEventLog.size() == 0) {
-        result.append("  [no events yet]\n");
+        dprintf(fd, "  [no events yet]\n");
     }
-    result.append("\n");
-
-    write(fd, result.string(), result.size());
+    dprintf(fd, "\n");
 }
 
 void CameraService::handleTorchClientBinderDied(const wp<IBinder> &who) {
diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
index f6ca903..2618838 100644
--- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
+++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
@@ -1285,37 +1285,34 @@
 }
 
 status_t CameraDeviceClient::dumpClient(int fd, const Vector<String16>& args) {
-    String8 result;
-    result.appendFormat("CameraDeviceClient[%s] (%p) dump:\n",
+    dprintf(fd, "  CameraDeviceClient[%s] (%p) dump:\n",
             mCameraIdStr.string(),
             (getRemoteCallback() != NULL ?
                     IInterface::asBinder(getRemoteCallback()).get() : NULL) );
-    result.appendFormat("  Current client UID %u\n", mClientUid);
+    dprintf(fd, "    Current client UID %u\n", mClientUid);
 
-    result.append("  State:\n");
-    result.appendFormat("    Request ID counter: %d\n", mRequestIdCounter);
+    dprintf(fd, "    State:\n");
+    dprintf(fd, "      Request ID counter: %d\n", mRequestIdCounter);
     if (mInputStream.configured) {
-        result.appendFormat("    Current input stream ID: %d\n",
-                    mInputStream.id);
+        dprintf(fd, "      Current input stream ID: %d\n", mInputStream.id);
     } else {
-        result.append("    No input stream configured.\n");
+        dprintf(fd, "      No input stream configured.\n");
     }
     if (!mStreamMap.isEmpty()) {
-        result.append("    Current output stream/surface IDs:\n");
+        dprintf(fd, "      Current output stream/surface IDs:\n");
         for (size_t i = 0; i < mStreamMap.size(); i++) {
-            result.appendFormat("      Stream %d Surface %d\n",
+            dprintf(fd, "        Stream %d Surface %d\n",
                                 mStreamMap.valueAt(i).streamId(),
                                 mStreamMap.valueAt(i).surfaceId());
         }
     } else if (!mDeferredStreams.isEmpty()) {
-        result.append("    Current deferred surface output stream IDs:\n");
+        dprintf(fd, "      Current deferred surface output stream IDs:\n");
         for (auto& streamId : mDeferredStreams) {
-            result.appendFormat("      Stream %d\n", streamId);
+            dprintf(fd, "        Stream %d\n", streamId);
         }
     } else {
-        result.append("    No output streams configured.\n");
+        dprintf(fd, "      No output streams configured.\n");
     }
-    write(fd, result.string(), result.size());
     // TODO: print dynamic/request section from most recent requests
     mFrameProcessor->dump(fd, args);
 
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index f691dc1..a0a3383 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -301,7 +301,6 @@
 status_t CameraProviderManager::dump(int fd, const Vector<String16>& args) {
     std::lock_guard<std::mutex> lock(mInterfaceMutex);
 
-    dprintf(fd, "Available camera providers and devices:\n");
     for (auto& provider : mProviders) {
         provider->dump(fd, args);
     }
@@ -482,18 +481,46 @@
 }
 
 status_t CameraProviderManager::ProviderInfo::dump(int fd, const Vector<String16>&) const {
-    dprintf(fd, "    %s: %zu devices:\n", mProviderName.c_str(), mDevices.size());
+    dprintf(fd, "== Camera Provider HAL %s (v2.4) static info: %zu devices: ==\n",
+            mProviderName.c_str(), mDevices.size());
 
     for (auto& device : mDevices) {
-        dprintf(fd, "        %s: Resource cost: %d\n", device->mName.c_str(),
-                device->mResourceCost.resourceCost);
-        if (device->mResourceCost.conflictingDevices.size() > 0) {
-            dprintf(fd, "            Conflicting devices:\n");
+        dprintf(fd, "== Camera HAL device %s (v%d.%d) static information: ==\n", device->mName.c_str(),
+                device->mVersion.get_major(), device->mVersion.get_minor());
+        dprintf(fd, "  Resource cost: %d\n", device->mResourceCost.resourceCost);
+        if (device->mResourceCost.conflictingDevices.size() == 0) {
+            dprintf(fd, "  Conflicting devices: None\n");
+        } else {
+            dprintf(fd, "  Conflicting devices:\n");
             for (size_t i = 0; i < device->mResourceCost.conflictingDevices.size(); i++) {
-                dprintf(fd, "              %s\n",
+                dprintf(fd, "    %s\n",
                         device->mResourceCost.conflictingDevices[i].c_str());
             }
         }
+        dprintf(fd, "  API1 info:\n");
+        dprintf(fd, "    Has a flash unit: %s\n",
+                device->hasFlashUnit() ? "true" : "false");
+        hardware::CameraInfo info;
+        status_t res = device->getCameraInfo(&info);
+        if (res != OK) {
+            dprintf(fd, "   <Error reading camera info: %s (%d)>\n",
+                    strerror(-res), res);
+        } else {
+            dprintf(fd, "    Facing: %s\n",
+                    info.facing == hardware::CAMERA_FACING_BACK ? "Back" : "Front");
+            dprintf(fd, "    Orientation: %d\n", info.orientation);
+        }
+        CameraMetadata info2;
+        res = device->getCameraCharacteristics(&info2);
+        if (res == INVALID_OPERATION) {
+            dprintf(fd, "  API2 not directly supported\n");
+        } else if (res != OK) {
+            dprintf(fd, "  <Error reading camera characteristics: %s (%d)>\n",
+                    strerror(-res), res);
+        } else {
+            dprintf(fd, "  API2 camera characteristics:\n");
+            info2.dump(fd, /*verbosity*/ 2, /*indentation*/ 4);
+        }
     }
     return OK;
 }
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 977e7a8..208dcb6 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -750,7 +750,7 @@
     mTagMonitor.dumpMonitoredMetadata(fd);
 
     if (mInterface->valid()) {
-        lines = String8("    HAL device dump:\n");
+        lines = String8("     HAL device dump:\n");
         write(fd, lines.string(), lines.size());
         mInterface->dump(fd);
     }
diff --git a/services/camera/libcameraservice/utils/CameraTraces.cpp b/services/camera/libcameraservice/utils/CameraTraces.cpp
index 374dc5e..0198690 100644
--- a/services/camera/libcameraservice/utils/CameraTraces.cpp
+++ b/services/camera/libcameraservice/utils/CameraTraces.cpp
@@ -74,7 +74,7 @@
         return BAD_VALUE;
     }
 
-    dprintf(fd, "Camera traces (%zu):\n", pcsList.size());
+    dprintf(fd, "== Camera error traces (%zu): ==\n", pcsList.size());
 
     if (pcsList.empty()) {
         dprintf(fd, "  No camera traces collected.\n");