ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

batterymonitor: simplify readFromFile and use std::string buffers

Browse source 
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>
This commit is contained in:
Michael Scott 2016-06-05 11:20:13 -07:00
parent 863d8e11b9
commit 3217c5c7d9
3 changed files with 57 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
healthd/Android.mk
View file

@ -17,7 +17,7 @@ LOCAL_SRC_FILES := BatteryMonitor.cpp
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 @@ endif
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

134
healthd/BatteryMonitor.cpp
View file

@ -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 @@ int BatteryMonitor::getBatteryHealth(const char* status) {
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 @@ BatteryMonitor::PowerSupplyType BatteryMonitor::readPowerSupplyType(const String
{ 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 @@ BatteryMonitor::PowerSupplyType BatteryMonitor::readPowerSupplyType(const String
}
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 @@ bool BatteryMonitor::update(void) {
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 @@ bool BatteryMonitor::update(void) {
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 @@ bool BatteryMonitor::update(void) {
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;
}

2
healthd/include/healthd/BatteryMonitor.h
View file

@ -55,7 +55,7 @@ class BatteryMonitor {
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);