From b7788fd454da2201c6e48dededa9b11873ef621e Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 28 Feb 2017 09:54:36 -0800 Subject: [PATCH] There's no longer a limit to property names. Bug: http://b/33926793 Test: boots Change-Id: I8554d7af74e064c114cf817f5a2ba1247fa2a2db --- .../libdebuggerd/test/sys/system_properties.h | 1 - init/service.cpp | 12 ++--- libsysutils/Android.mk | 2 +- libsysutils/src/ServiceManager.cpp | 52 ++++++------------- 4 files changed, 21 insertions(+), 46 deletions(-) diff --git a/debuggerd/libdebuggerd/test/sys/system_properties.h b/debuggerd/libdebuggerd/test/sys/system_properties.h index 9d4434530..1f4f58afc 100644 --- a/debuggerd/libdebuggerd/test/sys/system_properties.h +++ b/debuggerd/libdebuggerd/test/sys/system_properties.h @@ -32,7 +32,6 @@ // This is just enough to get the property code to compile on // the host. -#define PROP_NAME_MAX 32 #define PROP_VALUE_MAX 92 #endif // _DEBUGGERD_TEST_SYS_SYSTEM_PROPERTIES_H diff --git a/init/service.cpp b/init/service.cpp index e186f27a8..ba901fdce 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -180,12 +180,6 @@ void Service::NotifyStateChange(const std::string& new_state) const { } std::string prop_name = StringPrintf("init.svc.%s", name_.c_str()); - if (prop_name.length() >= PROP_NAME_MAX) { - // If the property name would be too long, we can't set it. - LOG(ERROR) << "Property name \"init.svc." << name_ << "\" too long; not setting to " << new_state; - return; - } - property_set(prop_name.c_str(), new_state.c_str()); if (new_state == "running") { @@ -1040,5 +1034,9 @@ void ServiceParser::EndSection() { } bool ServiceParser::IsValidName(const std::string& name) const { - return is_legal_property_name("init.svc." + name); + // Property names can be any length, but may only contain certain characters. + // Property values can contain any characters, but may only be a certain length. + // (The latter restriction is needed because `start` and `stop` work by writing + // the service name to the "ctl.start" and "ctl.stop" properties.) + return is_legal_property_name("init.svc." + name) && name.size() <= PROP_VALUE_MAX; } diff --git a/libsysutils/Android.mk b/libsysutils/Android.mk index 330d6cbf1..584e5a290 100644 --- a/libsysutils/Android.mk +++ b/libsysutils/Android.mk @@ -17,6 +17,7 @@ LOCAL_MODULE:= libsysutils LOCAL_CFLAGS := -Werror LOCAL_SHARED_LIBRARIES := \ + libbase \ libcutils \ liblog \ libnl @@ -24,4 +25,3 @@ LOCAL_SHARED_LIBRARIES := \ LOCAL_EXPORT_C_INCLUDE_DIRS := system/core/libsysutils/include include $(BUILD_SHARED_LIBRARY) - diff --git a/libsysutils/src/ServiceManager.cpp b/libsysutils/src/ServiceManager.cpp index 13bac0923..c7aa1f706 100644 --- a/libsysutils/src/ServiceManager.cpp +++ b/libsysutils/src/ServiceManager.cpp @@ -19,34 +19,23 @@ #include #include #include +#include #include -#include +#include +#include #include #include ServiceManager::ServiceManager() { } -/* The service name should not exceed SERVICE_NAME_MAX to avoid - * some weird things. This is due to the fact that: - * - * - Starting a service is done by writing its name to the "ctl.start" - * system property. This triggers the init daemon to actually start - * the service for us. - * - * - Stopping the service is done by writing its name to "ctl.stop" - * in a similar way. - * - * - Reading the status of a service is done by reading the property - * named "init.svc." - * - * If strlen() > (PROPERTY_KEY_MAX-1)-9, then you can start/stop - * the service by writing to ctl.start/stop, but you won't be able to - * read its state due to the truncation of "init.svc." into a - * zero-terminated buffer of PROPERTY_KEY_MAX characters. - */ -#define SERVICE_NAME_MAX (PROPERTY_KEY_MAX-10) +// The length of a service name should not exceed SERVICE_NAME_MAX. Starting +// a service is done by writing its name to the "ctl.start" system property +// and stopping a service is done by writing its name to "ctl.stop". If a +// service name is too long to fit in a property, you won't be able to start +// or stop it. +static constexpr size_t SERVICE_NAME_MAX = PROP_VALUE_MAX; /* The maximum amount of time to wait for a service to start or stop, * in micro-seconds (really an approximation) */ @@ -61,13 +50,14 @@ int ServiceManager::start(const char *name) { SLOGE("Service name '%s' is too long", name); return 0; } + if (isRunning(name)) { SLOGW("Service '%s' is already running", name); return 0; } SLOGD("Starting service '%s'", name); - property_set("ctl.start", name); + android::base::SetProperty("ctl.start", name); int count = SLEEP_MAX_USEC; while(count > 0) { @@ -90,13 +80,14 @@ int ServiceManager::stop(const char *name) { SLOGE("Service name '%s' is too long", name); return 0; } + if (!isRunning(name)) { SLOGW("Service '%s' is already stopped", name); return 0; } SLOGD("Stopping service '%s'", name); - property_set("ctl.stop", name); + android::base::SetProperty("ctl.stop", name); int count = SLEEP_MAX_USEC; while(count > 0) { @@ -116,19 +107,6 @@ int ServiceManager::stop(const char *name) { } bool ServiceManager::isRunning(const char *name) { - char propVal[PROPERTY_VALUE_MAX]; - char propName[PROPERTY_KEY_MAX]; - int ret; - - ret = snprintf(propName, sizeof(propName), "init.svc.%s", name); - if (ret > (int)sizeof(propName)-1) { - SLOGD("Service name '%s' is too long", name); - return false; - } - - if (property_get(propName, propVal, NULL)) { - if (!strcmp(propVal, "running")) - return true; - } - return false; + std::string property_name = android::base::StringPrintf("init.svc.%s", name); + return (android::base::GetProperty(property_name, "") == "running"); }