batterymonitor: simplify readFromFile and use std::string buffers
In readFromFile() when a newline is not found in the data, we reset
the initial character of the buffer to \0, but leave the count as is
(something >0 in this case).
Later in getBooleanField() we could erroneously treat a response as
"true" because count would be >0 and the initial value of buf would
be != '0' (set to \0 in this case).
To fixup error paths such as this, we can simplify readFromFile
by using android::base functions: ReadFromFileString() and Trim().
NOTES:
- Converted char * buffers used with readFromFile to std::string
- Removed unused variable btech from BatteryMonitor::update
Testing Done:
- Build healthd and recovery for angler device
- Confirm that known values are being read correctly from kernel
sysfs.
Change-Id: I238bbff097543767f352aa084bf0acbc1324baca
Signed-off-by: Michael Scott <michael.scott@linaro.org>
diff --git a/healthd/Android.mk b/healthd/Android.mk
index 127f39e..deebed5 100644
--- a/healthd/Android.mk
+++ b/healthd/Android.mk
@@ -17,7 +17,7 @@
LOCAL_MODULE := libbatterymonitor
LOCAL_C_INCLUDES := $(LOCAL_PATH)/include
LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
-LOCAL_STATIC_LIBRARIES := libutils libbinder
+LOCAL_STATIC_LIBRARIES := libutils libbase libbinder
include $(BUILD_STATIC_LIBRARY)
include $(CLEAR_VARS)
@@ -60,7 +60,7 @@
LOCAL_C_INCLUDES := bootable/recovery $(LOCAL_PATH)/include
-LOCAL_STATIC_LIBRARIES := libbatterymonitor libbatteryservice libbinder
+LOCAL_STATIC_LIBRARIES := libbatterymonitor libbatteryservice libbinder libbase
ifneq ($(strip $(LOCAL_CHARGER_NO_UI)),true)
LOCAL_STATIC_LIBRARIES += libminui libpng libz
diff --git a/healthd/BatteryMonitor.cpp b/healthd/BatteryMonitor.cpp
index 69647de..d1c547d 100644
--- a/healthd/BatteryMonitor.cpp
+++ b/healthd/BatteryMonitor.cpp
@@ -28,6 +28,8 @@
#include <unistd.h>
#include <memory>
+#include <android-base/file.h>
+#include <android-base/strings.h>
#include <batteryservice/BatteryService.h>
#include <cutils/klog.h>
#include <cutils/properties.h>
@@ -121,34 +123,15 @@
return ret;
}
-int BatteryMonitor::readFromFile(const String8& path, char* buf, size_t size) {
- char *cp = NULL;
-
- if (path.isEmpty())
- return -1;
- int fd = open(path.string(), O_RDONLY, 0);
- if (fd == -1) {
- KLOG_ERROR(LOG_TAG, "Could not open '%s'\n", path.string());
- return -1;
+int BatteryMonitor::readFromFile(const String8& path, std::string* buf) {
+ if (android::base::ReadFileToString(String8::std_string(path), buf)) {
+ *buf = android::base::Trim(*buf);
}
-
- ssize_t count = TEMP_FAILURE_RETRY(read(fd, buf, size));
- if (count > 0)
- cp = (char *)memrchr(buf, '\n', count);
-
- if (cp)
- *cp = '\0';
- else
- buf[0] = '\0';
-
- close(fd);
- return count;
+ return buf->length();
}
BatteryMonitor::PowerSupplyType BatteryMonitor::readPowerSupplyType(const String8& path) {
- const int SIZE = 128;
- char buf[SIZE];
- int length = readFromFile(path, buf, SIZE);
+ std::string buf;
BatteryMonitor::PowerSupplyType ret;
struct sysfsStringEnumMap supplyTypeMap[] = {
{ "Unknown", ANDROID_POWER_SUPPLY_TYPE_UNKNOWN },
@@ -167,12 +150,12 @@
{ NULL, 0 },
};
- if (length <= 0)
+ if (readFromFile(path, &buf) <= 0)
return ANDROID_POWER_SUPPLY_TYPE_UNKNOWN;
- ret = (BatteryMonitor::PowerSupplyType)mapSysfsString(buf, supplyTypeMap);
+ ret = (BatteryMonitor::PowerSupplyType)mapSysfsString(buf.c_str(), supplyTypeMap);
if (ret < 0) {
- KLOG_WARNING(LOG_TAG, "Unknown power supply type '%s'\n", buf);
+ KLOG_WARNING(LOG_TAG, "Unknown power supply type '%s'\n", buf.c_str());
ret = ANDROID_POWER_SUPPLY_TYPE_UNKNOWN;
}
@@ -180,27 +163,23 @@
}
bool BatteryMonitor::getBooleanField(const String8& path) {
- const int SIZE = 16;
- char buf[SIZE];
-
+ std::string buf;
bool value = false;
- if (readFromFile(path, buf, SIZE) > 0) {
- if (buf[0] != '0') {
+
+ if (readFromFile(path, &buf) > 0)
+ if (buf[0] != '0')
value = true;
- }
- }
return value;
}
int BatteryMonitor::getIntField(const String8& path) {
- const int SIZE = 128;
- char buf[SIZE];
-
+ std::string buf;
int value = 0;
- if (readFromFile(path, buf, SIZE) > 0) {
- value = strtol(buf, NULL, 0);
- }
+
+ if (readFromFile(path, &buf) > 0)
+ value = std::stoi(buf.c_str(), NULL, 0);
+
return value;
}
@@ -241,18 +220,16 @@
props.batteryHealth = BATTERY_HEALTH_GOOD;
}
- const int SIZE = 128;
- char buf[SIZE];
- String8 btech;
+ std::string buf;
- if (readFromFile(mHealthdConfig->batteryStatusPath, buf, SIZE) > 0)
- props.batteryStatus = getBatteryStatus(buf);
+ if (readFromFile(mHealthdConfig->batteryStatusPath, &buf) > 0)
+ props.batteryStatus = getBatteryStatus(buf.c_str());
- if (readFromFile(mHealthdConfig->batteryHealthPath, buf, SIZE) > 0)
- props.batteryHealth = getBatteryHealth(buf);
+ if (readFromFile(mHealthdConfig->batteryHealthPath, &buf) > 0)
+ props.batteryHealth = getBatteryHealth(buf.c_str());
- if (readFromFile(mHealthdConfig->batteryTechnologyPath, buf, SIZE) > 0)
- props.batteryTechnology = String8(buf);
+ if (readFromFile(mHealthdConfig->batteryTechnologyPath, &buf) > 0)
+ props.batteryTechnology = String8(buf.c_str());
unsigned int i;
@@ -261,33 +238,31 @@
path.appendFormat("%s/%s/online", POWER_SUPPLY_SYSFS_PATH,
mChargerNames[i].string());
- if (readFromFile(path, buf, SIZE) > 0) {
- if (buf[0] != '0') {
- path.clear();
- path.appendFormat("%s/%s/type", POWER_SUPPLY_SYSFS_PATH,
- mChargerNames[i].string());
- switch(readPowerSupplyType(path)) {
- case ANDROID_POWER_SUPPLY_TYPE_AC:
- props.chargerAcOnline = true;
- break;
- case ANDROID_POWER_SUPPLY_TYPE_USB:
- props.chargerUsbOnline = true;
- break;
- case ANDROID_POWER_SUPPLY_TYPE_WIRELESS:
- props.chargerWirelessOnline = true;
- break;
- default:
- KLOG_WARNING(LOG_TAG, "%s: Unknown power supply type\n",
- mChargerNames[i].string());
- }
- path.clear();
- path.appendFormat("%s/%s/current_max", POWER_SUPPLY_SYSFS_PATH,
- mChargerNames[i].string());
- if (access(path.string(), R_OK) == 0) {
- int maxChargingCurrent = getIntField(path);
- if (props.maxChargingCurrent < maxChargingCurrent) {
- props.maxChargingCurrent = maxChargingCurrent;
- }
+ if (getIntField(path)) {
+ path.clear();
+ path.appendFormat("%s/%s/type", POWER_SUPPLY_SYSFS_PATH,
+ mChargerNames[i].string());
+ switch(readPowerSupplyType(path)) {
+ case ANDROID_POWER_SUPPLY_TYPE_AC:
+ props.chargerAcOnline = true;
+ break;
+ case ANDROID_POWER_SUPPLY_TYPE_USB:
+ props.chargerUsbOnline = true;
+ break;
+ case ANDROID_POWER_SUPPLY_TYPE_WIRELESS:
+ props.chargerWirelessOnline = true;
+ break;
+ default:
+ KLOG_WARNING(LOG_TAG, "%s: Unknown power supply type\n",
+ mChargerNames[i].string());
+ }
+ path.clear();
+ path.appendFormat("%s/%s/current_max", POWER_SUPPLY_SYSFS_PATH,
+ mChargerNames[i].string());
+ if (access(path.string(), R_OK) == 0) {
+ int maxChargingCurrent = getIntField(path);
+ if (props.maxChargingCurrent < maxChargingCurrent) {
+ props.maxChargingCurrent = maxChargingCurrent;
}
}
}
@@ -343,10 +318,9 @@
int BatteryMonitor::getChargeStatus() {
int result = BATTERY_STATUS_UNKNOWN;
if (!mHealthdConfig->batteryStatusPath.isEmpty()) {
- char buf[128];
- if (readFromFile(mHealthdConfig->batteryStatusPath, buf, sizeof(buf)) > 0) {
- result = getBatteryStatus(buf);
- }
+ std::string buf;
+ if (readFromFile(mHealthdConfig->batteryStatusPath, &buf) > 0)
+ result = getBatteryStatus(buf.c_str());
}
return result;
}
diff --git a/healthd/include/healthd/BatteryMonitor.h b/healthd/include/healthd/BatteryMonitor.h
index 440f2e4..8865a7d 100644
--- a/healthd/include/healthd/BatteryMonitor.h
+++ b/healthd/include/healthd/BatteryMonitor.h
@@ -55,7 +55,7 @@
int getBatteryStatus(const char* status);
int getBatteryHealth(const char* status);
- int readFromFile(const String8& path, char* buf, size_t size);
+ int readFromFile(const String8& path, std::string* buf);
PowerSupplyType readPowerSupplyType(const String8& path);
bool getBooleanField(const String8& path);
int getIntField(const String8& path);