From 209f5deec3ee7a9759f9c3d68218b2df6e119336 Mon Sep 17 00:00:00 2001 From: "Christopher N. Hesse" Date: Sat, 4 Mar 2017 15:16:56 +0100 Subject: [PATCH] lights: Cleanup read/write API * Use O_WRONLY for write * Return errno for write * Log all failures * Fix read_int(): We would always treat strtol(...) = 0 as an error, although the real value can very well be 0. Check the end pointer to fix this. Change-Id: Id2bc9acc103a7a6400dd06f3904af37cbb0b5174 --- liblights/lights_helper.c | 74 ++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/liblights/lights_helper.c b/liblights/lights_helper.c index 5fb4af13..565101b1 100644 --- a/liblights/lights_helper.c +++ b/liblights/lights_helper.c @@ -28,38 +28,44 @@ * Reads an Integer from a file. * * @param path The absolute path string. - * @return The Integer with decimal base, -1 on error. + * @return The Integer with decimal base, -1 or errno on error. */ int read_int(char const *path) { int fd, len; int num_bytes = 10; char buf[11]; - int retval; + int ret; fd = open(path, O_RDONLY); if (fd < 0) { - ALOGE("%s: failed to open %s", __func__, path); - goto fail; + ret = errno; + ALOGE("%s: failed to open %s (%s)", __func__, path, strerror(errno)); + goto exit; } len = read(fd, buf, num_bytes - 1); if (len < 0) { - ALOGE("%s: failed to read from %s", __func__, path); - goto fail; + ret = errno; + ALOGE("%s: failed to read from %s (%s)", __func__, path, strerror(errno)); + goto exit; } buf[len] = '\0'; - close(fd); - // no endptr, decimal base - retval = strtol(buf, NULL, 10); - return retval == 0 ? -1 : retval; + // decimal base + char *endptr = NULL; + ret = strtol(buf, &endptr, 10); + if (ret == 0 && endptr == NULL) { + ret = -1; + ALOGE("%s: failed to extract number from string %s", __func__, buf); + goto exit; + } -fail: +exit: if (fd >= 0) close(fd); - return -1; + return ret; } /* @@ -67,38 +73,40 @@ fail: * * @param path The absolute path string. * @param value The Integer value to be written. - * @return 0 on success, -1 or errno on error. + * @return 0 on success, errno on error. */ int write_int(char const *path, const int value) { - int fd; - static int already_warned; + int fd, len; + int ret = 0; - already_warned = 0; + fd = open(path, O_WRONLY); + if (fd < 0) { + ret = errno; + ALOGE("%s: failed to open %s (%s)", __func__, path, strerror(errno)); + goto exit; + } - ALOGV("write_int: path %s, value %d", path, value); - fd = open(path, O_RDWR); + char buffer[20]; + int bytes = sprintf(buffer, "%d", value); + len = write(fd, buffer, bytes); + if (len < 0) { + ret = errno; + ALOGE("%s: failed to write to %s (%s)", __func__, path, strerror(errno)); + goto exit; + } - if (fd >= 0) { - char buffer[20]; - int bytes = sprintf(buffer, "%d", value); - int amt = write(fd, buffer, bytes); +exit: + if (fd >= 0) close(fd); - return amt == -1 ? -errno : 0; - } else { - if (already_warned == 0) { - ALOGE("write_int failed to open %s", path); - already_warned = 1; - } - return -errno; - } + return ret; } /* * Set the current button brightness via sysfs. * * @param brightness The brightness value. - * @return 0 on success, -1 or errno on error. + * @return 0 on success, errno on error. */ inline int set_cur_button_brightness(const int brightness) { @@ -129,7 +137,7 @@ inline int get_max_panel_brightness() * Set the current panel brightness via sysfs. * * @param brightness The (scaled) brightness value. - * @return 0 on success, -1 or errno on error. + * @return 0 on success, errno on error. */ inline int set_cur_panel_brightness(const int brightness) { @@ -140,7 +148,7 @@ inline int set_cur_panel_brightness(const int brightness) * Set the maximum panel brightness via sysfs. * * @param brightness The brightness value. - * @return 0 on success, -1 or errno on error. + * @return 0 on success, errno on error. */ inline int set_max_panel_brightness(const int brightness) {