From 0cedae398a108e287edb440ddcf877fb58bcb272 Mon Sep 17 00:00:00 2001
From: Miciah Masters <miciah.masters@gmail.com>
Date: Sat, 24 Dec 2016 03:57:47 -0500
Subject: Clean up error handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make several changes to error handling:

• Check for negative values from sprintf, and treat them as errors.

• Check for errors from fscanf both by comparing its return value against
  EOF and by checking ferror.

• Avoiding calling perror with errno set to -1.  Either set a meaningful
  value, such as EINVAL, or use fprintf if errno cannot be expected to
  have a meaningful value.

• Instead of re-using errno for internal error handling, use a new variable
  to indicate errors where it makes sense to do so.

• In read_device, print more specific error messages if we fail to read
  a device's current or max brightness, and print an error message
  immediately if a read fails, instead of only maybe at the end of
  read_device.
---
 brightnessctl.c | 52 ++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

(limited to 'brightnessctl.c')

diff --git a/brightnessctl.c b/brightnessctl.c
index 1eee6c6..55352d2 100644
--- a/brightnessctl.c
+++ b/brightnessctl.c
@@ -292,8 +292,8 @@ int write_device(struct device *d) {
 	char c[16];
 	size_t s = sprintf(c, "%u", d->curr_brightness);
 	errno = 0;
-	if (!s) {
-		errno = -1;
+	if (s <= 0) {
+		errno = EINVAL;
 		goto fail;
 	}
 	if ((f = fopen(dir_child(device_path(d), "brightness"), "w"))) {
@@ -314,7 +314,7 @@ int read_device(struct device *d, char *class, char *id) {
 	DIR *dirp;
 	FILE *f;
 	char *dev_path;
-	int done_read = 0;
+	int error = 0;
 	struct dirent *ent;
 	d->class = strdup(class);
 	d->id = strdup(id);
@@ -326,14 +326,28 @@ int read_device(struct device *d, char *class, char *id) {
 			continue;
 		if (!strcmp(ent->d_name, "brightness")) {
 			if ((f = fopen(dir_child(dev_path, ent->d_name), "r"))) {
-				done_read += fscanf(f, "%u", &d->curr_brightness);
+				clearerr(f);
+				if (fscanf(f, "%u", &d->curr_brightness) == EOF) {
+					fprintf(stderr, "End-of-file reading brightness of device '%s'.", d->id);
+					error++;
+				} else if (ferror(f)) {
+					fprintf(stderr, "Error reading brightness of device '%s': %s.", d->id, strerror(errno));
+					error++;
+				}
 				fclose(f);
 			} else
 				goto fail;
 		}
 		if (!strcmp(ent->d_name, "max_brightness")) {
 			if ((f = fopen(dir_child(dev_path, ent->d_name), "r"))) {
-				done_read += fscanf(f, "%u", &d->max_brightness);
+				clearerr(f);
+				if (fscanf(f, "%u", &d->max_brightness) == EOF) {
+					fprintf(stderr, "End-of-file reading max brightness of device '%s'.", d->id);
+					error++;
+				} else if (ferror(f)) {
+					fprintf(stderr, "Error reading max brightness of device '%s': %s.", d->id, strerror(errno));
+					error++;
+				}
 				fclose(f);
 			} else
 				goto fail;
@@ -342,11 +356,11 @@ int read_device(struct device *d, char *class, char *id) {
 	errno = 0;
 fail:
 	closedir(dirp);
-	if (!errno && !done_read)
-		errno = -1;
-	if (errno)
+	if (errno) {
 		perror("Error reading device");
-	return errno;
+		error++;
+	}
+	return error;
 }
 
 int read_class(struct device **devs, char *class) {
@@ -385,9 +399,11 @@ int save_device_data(struct device *dev) {
 	char *c_path = dir_child(run_dir, dev->class);
 	char *d_path = dir_child(c_path, dev->id);
 	FILE *fp;
+	int error = 0;
 	errno = 0;
-	if (!s) {
-		errno = -1;
+	if (s <= 0) {
+		fprintf(stderr, "Error converting device data.");
+		error++;
 		goto fail;
 	}
 	if (!ensure_run_dir())
@@ -405,15 +421,19 @@ int save_device_data(struct device *dev) {
 		goto fail;
 	if (!(fp = fopen(d_path, "w")))
 		goto fail;
-	if (fwrite(c, 1, s, fp) < s)
-		errno = -1;
+	if (fwrite(c, 1, s, fp) < s) {
+		fprintf(stderr, "Error writing to '%s'.\n", d_path);
+		error++;
+	}
 	fclose(fp);
 fail:
 	free(c_path);
 	free(d_path);
-	if (errno)
+	if (errno) {
 		perror("Error saving device data");
-	return errno;
+		error++;
+	}
+	return error;
 }
 
 int restore_device_data(struct device *dev) {
@@ -429,7 +449,7 @@ int restore_device_data(struct device *dev) {
 	fclose(fp);
 	dev->curr_brightness = strtol(buf, &end, 10);
 	if (end == buf)
-		errno = -1;
+		errno = EINVAL;
 fail:
 	if (errno) {
 		perror("Error restoring device data");
-- 
cgit v1.2.3