[devmgr] Restrict scope of dmctl_socket
Move dmctl_socket from being a global variable, to a member of
Coordinator.
Test: Ran Fuchsia and /system/test/ddk tests.
Change-Id: Ie54f5932c47dbc686979c0e31d0aee1386131add
diff --git a/system/core/devmgr/devmgr/coordinator.cpp b/system/core/devmgr/devmgr/coordinator.cpp
index b965024..4686f07 100644
--- a/system/core/devmgr/devmgr/coordinator.cpp
+++ b/system/core/devmgr/devmgr/coordinator.cpp
@@ -146,10 +146,8 @@
return ZX_OK;
}
-static zx::socket dmctl_socket;
-
-static void dmprintf(const char* fmt, ...) {
- if (!dmctl_socket.is_valid()) {
+void Coordinator::DmPrintf(const char* fmt, ...) const {
+ if (!dmctl_socket_.is_valid()) {
return;
}
char buf[1024];
@@ -158,8 +156,9 @@
vsnprintf(buf, sizeof(buf), fmt, ap);
va_end(ap);
size_t actual;
- if (dmctl_socket.write(0, buf, strlen(buf), &actual) != ZX_OK) {
- dmctl_socket.reset();
+ zx_status_t status = dmctl_socket_.write(0, buf, strlen(buf), &actual);
+ if (status != ZX_OK) {
+ dmctl_socket_.reset();
}
}
@@ -170,7 +169,7 @@
return ZX_OK;
}
if (!memcmp(cmd, "help", 4)) {
- dmprintf("dump - dump device tree\n"
+ DmPrintf("dump - dump device tree\n"
"poweroff - power off the system\n"
"shutdown - power off the system\n"
"suspend - suspend the system to RAM\n"
@@ -247,7 +246,7 @@
load_driver(path, fit::bind_member(this, &Coordinator::DriverAdded));
return ZX_OK;
}
- dmprintf("unknown command\n");
+ DmPrintf("unknown command\n");
log(ERROR, "dmctl: unknown command '%.*s'\n", (int) len, cmd);
return ZX_ERR_NOT_SUPPORTED;
}
@@ -322,9 +321,9 @@
extra[0] = 0;
}
if (pid == 0) {
- dmprintf("%*s[%s]%s\n", (int) (indent * 3), "", dev->name, extra);
+ DmPrintf("%*s[%s]%s\n", (int) (indent * 3), "", dev->name, extra);
} else {
- dmprintf("%*s%c%s%c pid=%zu%s %s\n",
+ DmPrintf("%*s%c%s%c pid=%zu%s %s\n",
(int) (indent * 3), "",
dev->flags & DEV_CTX_PROXY ? '<' : '[',
dev->name,
@@ -350,12 +349,12 @@
void Coordinator::DumpDeviceProps(const Device* dev) const {
if (dev->host) {
- dmprintf("Name [%s]%s%s%s\n",
+ DmPrintf("Name [%s]%s%s%s\n",
dev->name,
dev->libname ? " Driver [" : "",
dev->libname ? dev->libname : "",
dev->libname ? "]" : "");
- dmprintf("Flags :%s%s%s%s%s%s%s\n",
+ DmPrintf("Flags :%s%s%s%s%s%s%s\n",
dev->flags & DEV_CTX_IMMORTAL ? " Immortal" : "",
dev->flags & DEV_CTX_MUST_ISOLATE ? " Isolate" : "",
dev->flags & DEV_CTX_MULTI_BIND ? " MultiBind" : "",
@@ -368,7 +367,7 @@
char b = (char)((dev->protocol_id >> 16) & 0xFF);
char c = (char)((dev->protocol_id >> 8) & 0xFF);
char d = (char)(dev->protocol_id & 0xFF);
- dmprintf("ProtoId : '%c%c%c%c' 0x%08x(%u)\n",
+ DmPrintf("ProtoId : '%c%c%c%c' 0x%08x(%u)\n",
isprint(a) ? a : '.',
isprint(b) ? b : '.',
isprint(c) ? c : '.',
@@ -376,20 +375,20 @@
dev->protocol_id,
dev->protocol_id);
- dmprintf("%u Propert%s\n", dev->prop_count, dev->prop_count == 1 ? "y" : "ies");
+ DmPrintf("%u Propert%s\n", dev->prop_count, dev->prop_count == 1 ? "y" : "ies");
for (uint32_t i = 0; i < dev->prop_count; ++i) {
const zx_device_prop_t* p = &dev->props[i];
const char* param_name = di_bind_param_name(p->id);
if (param_name) {
- dmprintf("[%2u/%2u] : Value 0x%08x Id %s\n",
+ DmPrintf("[%2u/%2u] : Value 0x%08x Id %s\n",
i, dev->prop_count, p->value, param_name);
} else {
- dmprintf("[%2u/%2u] : Value 0x%08x Id 0x%04hx\n",
+ DmPrintf("[%2u/%2u] : Value 0x%08x Id 0x%04hx\n",
i, dev->prop_count, p->value, p->id);
}
}
- dmprintf("\n");
+ DmPrintf("\n");
}
if (dev->proxy) {
@@ -410,17 +409,17 @@
void Coordinator::DumpDrivers() const {
bool first = true;
for (const auto& drv : drivers_) {
- dmprintf("%sName : %s\n", first ? "" : "\n", drv.name.c_str());
- dmprintf("Driver : %s\n", !drv.libname.empty() ? drv.libname.c_str() : "(null)");
- dmprintf("Flags : 0x%08x\n", drv.flags);
+ DmPrintf("%sName : %s\n", first ? "" : "\n", drv.name.c_str());
+ DmPrintf("Driver : %s\n", !drv.libname.empty() ? drv.libname.c_str() : "(null)");
+ DmPrintf("Flags : 0x%08x\n", drv.flags);
if (drv.binding_size) {
char line[256];
uint32_t count = drv.binding_size / static_cast<uint32_t>(sizeof(drv.binding[0]));
- dmprintf("Binding : %u instruction%s (%u bytes)\n",
+ DmPrintf("Binding : %u instruction%s (%u bytes)\n",
count, (count == 1) ? "" : "s", drv.binding_size);
for (uint32_t i = 0; i < count; ++i) {
di_dump_bind_inst(&drv.binding[i], line, sizeof(line));
- dmprintf("[%u/%u]: %s\n", i + 1, count, line);
+ DmPrintf("[%u/%u]: %s\n", i + 1, count, line);
}
}
first = false;
@@ -1241,13 +1240,13 @@
const char* command_data, size_t command_size, fidl_txn_t* txn) {
zx::socket log_socket(raw_log_socket);
+ auto dev = static_cast<Device*>(ctx);
if (log_socket.is_valid()) {
- dmctl_socket = std::move(log_socket);
+ dev->coordinator->set_dmctl_socket(std::move(log_socket));
}
- auto dev = static_cast<Device*>(ctx);
zx_status_t status = dev->coordinator->HandleDmctlWrite(command_size, command_data);
- dmctl_socket.reset();
+ dev->coordinator->set_dmctl_socket(zx::socket());
return fuchsia_device_manager_CoordinatorDmCommand_reply(txn, status);
}
@@ -1897,7 +1896,7 @@
return;
}
// Move the socket in to prevent the rpc handler from closing the handle.
- *ctx = SuspendContext(this, SuspendContext::Flags::kSuspend, flags, std::move(dmctl_socket));
+ *ctx = SuspendContext(this, SuspendContext::Flags::kSuspend, flags, std::move(dmctl_socket_));
ctx->set_dh(BuildSuspendList(ctx));
diff --git a/system/core/devmgr/devmgr/coordinator.h b/system/core/devmgr/devmgr/coordinator.h
index 3a86075..59d5b93 100644
--- a/system/core/devmgr/devmgr/coordinator.h
+++ b/system/core/devmgr/devmgr/coordinator.h
@@ -419,6 +419,7 @@
zx_status_t InitializeCoreDevices();
+ void DmPrintf(const char* fmt, ...) const;
zx_status_t HandleDmctlWrite(size_t len, const char* cmd);
const Driver* LibnameToDriver(const char* libname) const;
@@ -487,6 +488,7 @@
void set_loader_service(DevhostLoaderService* loader_service) {
loader_service_ = loader_service;
}
+ void set_dmctl_socket(zx::socket dmctl_socket) { dmctl_socket_ = std::move(dmctl_socket); }
fbl::DoublyLinkedList<Device*, Device::AllDevicesNode>& devices() { return devices_; }
Device& root_device() { return root_device_; }
@@ -511,6 +513,10 @@
bool running_ = false;
DevhostLoaderService* loader_service_ = nullptr;
+ // This socket is used by DmPrintf for output, and DmPrintf can be called in
+ // the context of a const member function, therefore it is also const. Given
+ // that, we must make dmctl_socket_ mutable.
+ mutable zx::socket dmctl_socket_;
zx::vmo bootdata_vmo_;
// All Drivers
