diff --git a/adapter/uhdf2/hdi/src/idevmgr_client.cpp b/adapter/uhdf2/hdi/src/idevmgr_client.cpp index 467fb0e1c5fd885ada50675ede9601f1f93e5932..2a6c9e532065a9b1d2f15ecf2fdc9b9447ae7d3f 100644 --- a/adapter/uhdf2/hdi/src/idevmgr_client.cpp +++ b/adapter/uhdf2/hdi/src/idevmgr_client.cpp @@ -30,6 +30,8 @@ namespace OHOS { namespace HDI { namespace DeviceManager { namespace V1_0 { +#define HDF_MAX_HOST_COUNT 0xFF + std::mutex g_remoteMutex; enum DevmgrCmdId : uint32_t { @@ -185,18 +187,29 @@ int32_t DeviceManagerProxy::ListAllDevice(std::vector &deviceInf return status; } -static void HdfDevMgrFillPidList(std::vector &pidList, MessageParcel &reply) +static int32_t HdfDevMgrFillPidList(std::vector &pidList, MessageParcel &reply) { - uint32_t count = 0; - reply.ReadUint32(count); + uint32_t pidCount = 0; + if (!reply.ReadUint32(pidCount)) { + HDF_LOGE("failed to read pid count"); + return HDF_FAILURE; + } + + if (pidCount >= HDF_MAX_HOST_COUNT) { + HDF_LOGE("invalid host count, %{public}d", pidCount); + return HDF_ERR_OUT_OF_RANGE; + } int pid = -1; - for (uint32_t i = 0; i < count; ++i) { - reply.ReadInt32(pid); + for (uint32_t i = 0; i < pidCount; ++i) { + if (!reply.ReadInt32(pid)) { + HDF_LOGE("failed to read pid"); + return HDF_FAILURE; + } pidList.push_back(pid); } - return; + return HDF_SUCCESS; } int32_t DeviceManagerProxy::ListAllHost(std::vector &pidList) @@ -217,13 +230,16 @@ int32_t DeviceManagerProxy::ListAllHost(std::vector &pidList) static_cast(DEVMGR_SERVICE_LIST_ALL_HOST), data, reply, option); lock.unlock(); if (status != HDF_SUCCESS) { - HDF_LOGE("list all service info failed, %{public}d", status); + HDF_LOGE("list all host failed, %{public}d", status); return status; - } else { - HdfDevMgrFillPidList(pidList, reply); } - HDF_LOGD("get all service info success"); - return status; + + int32_t ret = HdfDevMgrFillPidList(pidList, reply); + if (ret != HDF_SUCCESS) { + HDF_LOGE("fill pid list failed"); + } + + return ret; } sptr IDeviceManager::Get() diff --git a/adapter/uhdf2/host/src/devhost_dump.c b/adapter/uhdf2/host/src/devhost_dump.c index b9ef1d73cdd6f0592f6893404161eaa437f583ee..e39239a22c7523f2b000e8e040838990f34536cc 100644 --- a/adapter/uhdf2/host/src/devhost_dump.c +++ b/adapter/uhdf2/host/src/devhost_dump.c @@ -120,6 +120,25 @@ int32_t DevHostRegisterDumpHost(DevHostDumpFunc dump) return HDF_SUCCESS; } +static void DevHostDumpIpc(struct HdfSBuf *data) +{ + int fd = HdfSbufReadFileDescriptor(data); + if (fd < 0) { + HDF_LOGE("invalid fd %{public}d", fd); + return; + } + HDF_LOGI("%{public}s %{public}d", __func__, fd); + + const char *dumpCmd = HdfSbufReadString(data); + if (dumpCmd == NULL) { + HDF_LOGE("ipc dumpCmd is NULL"); + return; + } + + HdfDumpIpcStat(fd, dumpCmd); + return; +} + void DevHostDump(struct HdfSBuf *data, struct HdfSBuf *reply) { if (data == NULL || reply == NULL) { @@ -158,13 +177,7 @@ void DevHostDump(struct HdfSBuf *data, struct HdfSBuf *reply) (void)HdfSbufWriteString(reply, "The service does not register dump function\n"); } } else if (strcmp(option, "--ipc") == 0) { - int32_t fd = HdfSbufReadFileDescriptor(data); - HDF_LOGI("%{public}s %{public}d", option, fd); - const char *dumpCmd = HdfSbufReadString(data); - if (dumpCmd == NULL) { - return; - } - HdfDumpIpcStat(fd, dumpCmd); + DevHostDumpIpc(data); } else { HDF_LOGE("%{public}s invalid parameter %{public}s", __func__, option); } diff --git a/adapter/uhdf2/ipc/src/hdf_dump.cpp b/adapter/uhdf2/ipc/src/hdf_dump.cpp index 3cea8189294a5f954788c22f7608e39376e6a049..a64d2ee925f2c5e2470e748961573c61fba2c4b4 100644 --- a/adapter/uhdf2/ipc/src/hdf_dump.cpp +++ b/adapter/uhdf2/ipc/src/hdf_dump.cpp @@ -39,23 +39,23 @@ const char *HDF_DUMP_FAIL_STR = " fail\n"; // The maximum parameter is the parameter sent to the host, including public(num=2) and private(mux_num=20) parameters static constexpr int32_t MAX_PARA_NUM = 22; -static bool HdfDumpIpcStatStart(std::string& result) +static void HdfDumpIpcStatStart(std::string& result) { result = std::string("HdfDumpIpcStatStart pid:") + std::to_string(getpid()); bool ret = IPCPayloadStatistics::StartStatistics(); result += ret ? HDF_DUMP_SUCCESS_STR : HDF_DUMP_FAIL_STR; - return ret; + return; } -static int32_t HdfDumpIpcStatStop(std::string& result) +static void HdfDumpIpcStatStop(std::string& result) { result = std::string("HdfDumpIpcStatStop pid:") + std::to_string(getpid()); bool ret = IPCPayloadStatistics::StopStatistics(); result += ret ? HDF_DUMP_SUCCESS_STR : HDF_DUMP_FAIL_STR; - return ret; + return; } -static int32_t HdfDumpIpcStatGet(std::string& result) +static void HdfDumpIpcStatGet(std::string& result) { result += "********************************GlobalStatisticsInfo********************************"; result += "\nCurrentPid:"; @@ -64,72 +64,82 @@ static int32_t HdfDumpIpcStatGet(std::string& result) result += std::to_string(IPCPayloadStatistics::GetTotalCount()); result += "\nTotalTimeCost:"; result += std::to_string(IPCPayloadStatistics::GetTotalCost()); + std::vector pids; pids = IPCPayloadStatistics::GetPids(); for (unsigned int i = 0; i < pids.size(); i++) { + int32_t pid = pids[i]; result += "\n--------------------------------ProcessStatisticsInfo-------------------------------"; result += "\nCallingPid:"; - result += std::to_string(pids[i]); + result += std::to_string(pid); result += "\nCallingPidTotalCount:"; - result += std::to_string(IPCPayloadStatistics::GetCount(pids[i])); + result += std::to_string(IPCPayloadStatistics::GetCount(pid)); result += "\nCallingPidTotalTimeCost:"; - result += std::to_string(IPCPayloadStatistics::GetCost(pids[i])); + result += std::to_string(IPCPayloadStatistics::GetCost(pid)); + std::vector intfs; - intfs = IPCPayloadStatistics::GetDescriptorCodes(pids[i]); + intfs = IPCPayloadStatistics::GetDescriptorCodes(pid); for (unsigned int j = 0; j < intfs.size(); j++) { + IPCInterfaceInfo intf = intfs[j]; result += "\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~InterfaceStatisticsInfo~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"; result += "\nDescriptorCode:"; - result += Str16ToStr8(intfs[j].desc) + std::string("_") + std::to_string(intfs[j].code); + result += Str16ToStr8(intf.desc) + std::string("_") + std::to_string(intf.code); result += "\nDescriptorCodeCount:"; result += std::to_string( - IPCPayloadStatistics::GetDescriptorCodeCount(pids[i], intfs[j].desc, intfs[j].code)); + IPCPayloadStatistics::GetDescriptorCodeCount(pid, intf.desc, intf.code)); + result += "\nDescriptorCodeTimeCost:"; result += "\nTotal:"; result += std::to_string( - IPCPayloadStatistics::GetDescriptorCodeCost(pids[i], intfs[j].desc, intfs[j].code).totalCost); + IPCPayloadStatistics::GetDescriptorCodeCost(pid, intf.desc, intf.code).totalCost); result += " | Max:"; result += std::to_string( - IPCPayloadStatistics::GetDescriptorCodeCost(pids[i], intfs[j].desc, intfs[j].code).maxCost); + IPCPayloadStatistics::GetDescriptorCodeCost(pid, intf.desc, intf.code).maxCost); result += " | Min:"; result += std::to_string( - IPCPayloadStatistics::GetDescriptorCodeCost(pids[i], intfs[j].desc, intfs[j].code).minCost); + IPCPayloadStatistics::GetDescriptorCodeCost(pid, intf.desc, intf.code).minCost); result += " | Avg:"; result += std::to_string( - IPCPayloadStatistics::GetDescriptorCodeCost(pids[i], intfs[j].desc, intfs[j].code).averCost); + IPCPayloadStatistics::GetDescriptorCodeCost(pid, intf.desc, intf.code).averCost); result += "\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"; } result += "\n------------------------------------------------------------------------------------"; } result += "\n************************************************************************************\n"; - return true; + return; } -bool HdfDumpIpcStat(int32_t fd, const char *cmd) +void HdfDumpIpcStat(int32_t fd, const char *cmd) { + if (fd < 0) { + HDF_LOGE("invalid fd %{public}d", fd); + return; + } + if (cmd == nullptr) { HDF_LOGE("%{public}s cmd is null", __func__); - return HDF_FAILURE; + return; } - bool ret = false; std::string result; HDF_LOGI("%{public}s %{public}d", cmd, fd); if (strcmp(cmd, "--start-stat") == 0) { - ret = HdfDumpIpcStatStart(result); + HdfDumpIpcStatStart(result); } else if (strcmp(cmd, "--stop-stat") == 0) { - ret = HdfDumpIpcStatStop(result); + HdfDumpIpcStatStop(result); } else if (strcmp(cmd, "--stat") == 0) { - ret = HdfDumpIpcStatGet(result); + HdfDumpIpcStatGet(result); } else { - return ret; + HDF_LOGE("%{public}s invalid param", __func__); + return; } if (!OHOS::SaveStringToFd(fd, result)) { - ret = false; + HDF_LOGE("%{public}s SaveStringToFd failed", __func__); } - return ret; + return; } void HdfRegisterDumpFunc(DevHostDumpFunc dump) diff --git a/adapter/uhdf2/manager/src/devmgr_dump.c b/adapter/uhdf2/manager/src/devmgr_dump.c index e2eccf70c2bbc2d464b9e8d69eb615bd3f34f62c..ae7fbb28f3448aac9ee8cbc75481072ca32ce43f 100644 --- a/adapter/uhdf2/manager/src/devmgr_dump.c +++ b/adapter/uhdf2/manager/src/devmgr_dump.c @@ -192,7 +192,24 @@ static int32_t DevMgrDumpService(uint32_t argv, struct HdfSBuf *data, struct Hdf return ret; } -static int32_t DevMgrDumpAllHostIpcStats(struct HdfSBuf *data, struct HdfSBuf *reply) +static int32_t DevMgrDumpIpcFillHostCmd(struct HdfSBuf *ipcData, int32_t fd, const char *cmd) +{ + if (!HdfSbufWriteString(ipcData, "--ipc")) { + return HDF_FAILURE; + } + + if (!HdfSbufWriteFileDescriptor(ipcData, fd)) { + return HDF_FAILURE; + } + + if (!HdfSbufWriteString(ipcData, cmd)) { + return HDF_FAILURE; + } + + return HDF_SUCCESS; +} + +static int32_t DevMgrDumpAllHostsIpcStats(struct HdfSBuf *data, struct HdfSBuf *reply) { struct DevmgrService *devMgrSvc = (struct DevmgrService *)DevmgrServiceGetInstance(); if (devMgrSvc == NULL) { @@ -204,38 +221,32 @@ static int32_t DevMgrDumpAllHostIpcStats(struct HdfSBuf *data, struct HdfSBuf *r DLIST_FOR_EACH_ENTRY(hostClnt, &devMgrSvc->hosts, struct DevHostServiceClnt, node) { HDF_LOGI("%{public}s hostName:%{public}s", __func__, hostClnt->hostName); if (hostClnt->hostService == NULL || hostClnt->hostService->Dump == NULL) { - HDF_LOGI("The host does not start\n"); + HDF_LOGI("The host does not start"); continue; } ret = hostClnt->hostService->Dump(hostClnt->hostService, data, reply); + if (ret != HDF_SUCCESS) { + HDF_LOGE("Dump Host failed"); + } } return ret; } -static int32_t DevMgrDumpAllHostIpcStat(int32_t fd, const char *cmd, struct HdfSBuf *reply) +static int32_t DevMgrDumpIpcAllHosts(int32_t fd, const char *cmd, struct HdfSBuf *reply) { struct HdfSBuf *ipcData = HdfSbufTypedObtain(SBUF_IPC); if (ipcData == NULL) { return HDF_FAILURE; } - if (!HdfSbufWriteString(ipcData, "--ipc")) { - HdfSbufRecycle(ipcData); - return HDF_FAILURE; - } - - if (!HdfSbufWriteFileDescriptor(ipcData, fd)) { - HdfSbufRecycle(ipcData); - return HDF_FAILURE; - } - - if (!HdfSbufWriteString(ipcData, cmd)) { + int32_t ret = DevMgrDumpIpcFillHostCmd(ipcData, fd, cmd); + if (ret != HDF_SUCCESS) { HdfSbufRecycle(ipcData); return HDF_FAILURE; } - int32_t ret = DevMgrDumpAllHostIpcStats(ipcData, reply); + ret = DevMgrDumpAllHostsIpcStats(ipcData, reply); HdfSbufRecycle(ipcData); return ret; } @@ -251,7 +262,7 @@ static int32_t DevMgrDumpSingleHostIpcStats(int32_t pid, struct HdfSBuf *data, s int32_t ret = HDF_FAILURE; DLIST_FOR_EACH_ENTRY(hostClnt, &devMgrSvc->hosts, struct DevHostServiceClnt, node) { if (hostClnt->hostService == NULL || hostClnt->hostService->Dump == NULL) { - HDF_LOGI("The host does not start\n"); + HDF_LOGI("The host does not start"); continue; } if (hostClnt->hostProcessId == pid) { @@ -265,29 +276,20 @@ static int32_t DevMgrDumpSingleHostIpcStats(int32_t pid, struct HdfSBuf *data, s return ret; } -static int32_t DevMgrDumpSingleHostIpcStat(int32_t pid, int32_t fd, const char *cmd, struct HdfSBuf *reply) +static int32_t DevMgrDumpIpcSingleHost(int32_t pid, int32_t fd, const char *cmd, struct HdfSBuf *reply) { struct HdfSBuf *ipcData = HdfSbufTypedObtain(SBUF_IPC); if (ipcData == NULL) { return HDF_FAILURE; } - if (!HdfSbufWriteString(ipcData, "--ipc")) { - HdfSbufRecycle(ipcData); - return HDF_FAILURE; - } - - if (!HdfSbufWriteFileDescriptor(ipcData, fd)) { - HdfSbufRecycle(ipcData); - return HDF_FAILURE; - } - - if (!HdfSbufWriteString(ipcData, cmd)) { + int32_t ret = DevMgrDumpIpcFillHostCmd(ipcData, fd, cmd); + if (ret != HDF_SUCCESS) { HdfSbufRecycle(ipcData); return HDF_FAILURE; } - int32_t ret = DevMgrDumpSingleHostIpcStats(pid, ipcData, reply); + ret = DevMgrDumpSingleHostIpcStats(pid, ipcData, reply); HdfSbufRecycle(ipcData); return ret; } @@ -309,13 +311,13 @@ static int32_t DevMgrDumpIpc(int32_t fd, struct HdfSBuf *data, struct HdfSBuf *r HDF_LOGI("%{public}s %{public}s fd:%{public}d", value, dumpCmd, fd); if (strcmp(value, "all") == 0) { HdfDumpIpcStat(fd, dumpCmd); - return DevMgrDumpAllHostIpcStat(fd, dumpCmd, reply); + return DevMgrDumpIpcAllHosts(fd, dumpCmd, reply); } else { int32_t pid = atoi(value); if (pid == getpid()) { HdfDumpIpcStat(fd, dumpCmd); } else { - DevMgrDumpSingleHostIpcStat(pid, fd, dumpCmd, reply); + return DevMgrDumpIpcSingleHost(pid, fd, dumpCmd, reply); } } @@ -601,6 +603,10 @@ static int32_t DevMgrDump(struct HdfSBuf *data, struct HdfSBuf *reply) } int32_t fd = HdfSbufReadFileDescriptor(data); + if (fd < 0) { + HDF_LOGE("invalid fd %{public}d", fd); + return HDF_FAILURE; + } uint32_t argv = 0; HdfSbufReadUint32(data, &argv); diff --git a/adapter/uhdf2/manager/src/devmgr_service_stub.c b/adapter/uhdf2/manager/src/devmgr_service_stub.c index e4ec2609c78d4ea26db128001a199dd85f6e5aaa..33866475ba151476f89b8266253118a87ece311d 100644 --- a/adapter/uhdf2/manager/src/devmgr_service_stub.c +++ b/adapter/uhdf2/manager/src/devmgr_service_stub.c @@ -132,14 +132,21 @@ static int32_t DevmgrServiceStubDispatchListAllDevice(struct IDevmgrService *dev static int32_t DevmgrServiceStubListAllHost(struct IDevmgrService *devmgrSvc, struct HdfSBuf *reply) { if (reply == NULL) { - HDF_LOGE("%{public}s:service name is null", __func__); + HDF_LOGE("%{public}s:reply is null", __func__); return HDF_ERR_INVALID_PARAM; } - HDF_LOGD("%{public}s:get all device info", __func__); int32_t ret = devmgrSvc->ListAllHost(devmgrSvc, reply); - HdfSbufWriteInt32(reply, getpid()); - return ret; + if (ret != HDF_SUCCESS) { + HDF_LOGE("%{public}s ListAllHost failed ret = %{public}d", __func__, ret); + return ret; + } + + if (!HdfSbufWriteInt32(reply, getpid())) { + HDF_LOGE("%{public}s: Sbuf Write devmgr pid failed", __func__); + return HDF_FAILURE; + } + return HDF_SUCCESS; } int32_t DevmgrServiceStubDispatch(struct HdfRemoteService *stub, int code, struct HdfSBuf *data, struct HdfSBuf *reply) diff --git a/framework/core/manager/src/devmgr_service.c b/framework/core/manager/src/devmgr_service.c index ad72ae34e1bc9c84321290d983aeaa051fc9e926..48953a413f8813268dd90167397cbed0167c5643 100644 --- a/framework/core/manager/src/devmgr_service.c +++ b/framework/core/manager/src/devmgr_service.c @@ -408,9 +408,15 @@ static int32_t DevmgrServiceListAllHost(struct IDevmgrService *inst, struct HdfS return HDF_FAILURE; } - HdfSbufWriteUint32(reply, DListGetCount(&devMgrSvc->hosts) + 1); + if (!HdfSbufWriteUint32(reply, DListGetCount(&devMgrSvc->hosts) + 1)) { + HDF_LOGE("Sbuf Write host count failed"); + return HDF_FAILURE; + } DLIST_FOR_EACH_ENTRY(hostClnt, &devMgrSvc->hosts, struct DevHostServiceClnt, node) { - HdfSbufWriteInt32(reply, hostClnt->hostProcessId); + if (!HdfSbufWriteInt32(reply, hostClnt->hostProcessId)) { + HDF_LOGE("%{public}s: Sbuf Write host pid failed", __func__); + return HDF_FAILURE; + } } return HDF_SUCCESS; diff --git a/interfaces/inner_api/ipc/hdf_dump_reg.h b/interfaces/inner_api/ipc/hdf_dump_reg.h index d59d5929308dd90d335f9bcdea4fc2a24c42d811..11b67dc7ed3699e58c83c3cf07975248d1b94f06 100644 --- a/interfaces/inner_api/ipc/hdf_dump_reg.h +++ b/interfaces/inner_api/ipc/hdf_dump_reg.h @@ -44,7 +44,7 @@ extern "C" { #endif /* __cplusplus */ -bool HdfDumpIpcStat(int32_t fd, const char *cmd); +void HdfDumpIpcStat(int32_t fd, const char *cmd); /** * @brief Implements IPC dump.