From 76463088d1e46fbac0f477b38b55bdb79764061d Mon Sep 17 00:00:00 2001
From: Michael Orlitzky <michael@orlitzky.com>
Date: Mon, 26 Feb 2018 08:27:13 -0500
Subject: [PATCH] Define a few more error constants to make the code more
 readable.

---
 src/apply-default-acl.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/apply-default-acl.c b/src/apply-default-acl.c
index 45dfa56..188c1f2 100644
--- a/src/apply-default-acl.c
+++ b/src/apply-default-acl.c
@@ -33,6 +33,14 @@
 #define ACL_FAILURE 0
 #define ACL_SUCCESS 1
 
+/* Even though most other library functions reliably return -1 for
+ * error, it feels a little wrong to re-use the ACL_ERROR constant.
+ */
+#define CLOSE_ERROR -1
+#define NFTW_ERROR -1
+#define OPEN_ERROR -1
+#define SNPRINTF_ERROR -1
+#define STAT_ERROR -1
 
 int safe_open_ex(int at_fd, char* pathname, int flags) {
   if (pathname != NULL && strlen(pathname) == 0) {
@@ -51,21 +59,21 @@ int safe_open_ex(int at_fd, char* pathname, int flags) {
      openat() opens only the next directory (and doesn't recurse). */
    *firstslash = '\0';
    int fd = safe_open_ex(at_fd, pathname, flags);
-   if (fd == -1) {
+   if (fd == OPEN_ERROR) {
      if (errno != ELOOP) {
        /* Don't output anything if we ignore a symlink */
        perror("safe_open_ex (safe_open_ex)");
      }
-     return -1;
+     return OPEN_ERROR;
    }
 
    /* The ++ is safe because there needs to be at least a null byte
       after the first slash, even if it's the last real character in
       the string. */
    int result = safe_open_ex(fd, firstslash+1, flags);
-   if (close(fd) == -1) {
+   if (close(fd) == CLOSE_ERROR) {
       perror("safe_open_ex (close)");
-      return -1;
+      return OPEN_ERROR;
     }
    return result;
 }
@@ -74,7 +82,7 @@ int safe_open_ex(int at_fd, char* pathname, int flags) {
 int safe_open(const char* pathname, int flags) {
   if (pathname == NULL || strlen(pathname) == 0 || pathname[0] == '\0') {
     /* error? */
-    return -1;
+    return OPEN_ERROR;
   }
 
   char abspath[PATH_MAX];
@@ -97,21 +105,21 @@ int safe_open(const char* pathname, int flags) {
     char* cwd = get_current_dir_name();
     if (cwd == NULL) {
       perror("safe_open (get_current_dir_name)");
-      return -1;
+      return OPEN_ERROR;
     }
 
     char abs_cwd[PATH_MAX];
     if (realpath(cwd, abs_cwd) == NULL) {
       perror("safe_open (realpath)");
       free(cwd);
-      return -1;
+      return OPEN_ERROR;
     }
     snprintf_result = snprintf(abspath, PATH_MAX, "%s/%s", abs_cwd, pathname);
     free(cwd);
   }
-  if (snprintf_result == -1 || snprintf_result > PATH_MAX) {
+  if (snprintf_result == SNPRINTF_ERROR || snprintf_result > PATH_MAX) {
     perror("safe_open (snprintf)");
-    return -1;
+    return OPEN_ERROR;
   }
 
   int fd = open("/", flags);
@@ -120,9 +128,9 @@ int safe_open(const char* pathname, int flags) {
   }
 
   int result = safe_open_ex(fd, abspath+1, flags);
-  if (close(fd) == -1) {
+  if (close(fd) == CLOSE_ERROR) {
     perror("safe_open (close)");
-    return -1;
+    return OPEN_ERROR;
   }
   return result;
 }
@@ -641,7 +649,7 @@ int apply_default_acl(const char* path,
   char* parent = dirname(path_copy);
 
   fd = safe_open(path, O_NOFOLLOW);
-  if (fd == -1) {
+  if (fd == OPEN_ERROR) {
     if (errno == ELOOP) {
       result = ACL_FAILURE; /* hit a symlink */
       goto cleanup;
@@ -666,7 +674,7 @@ int apply_default_acl(const char* path,
   */
   if (sp == NULL) {
     struct stat s;
-    if (fstat(fd, &s) == -1) {
+    if (fstat(fd, &s) == STAT_ERROR) {
       perror("apply_default_acl (fstat)");
       goto cleanup;
     }
@@ -827,7 +835,7 @@ int apply_default_acl(const char* path,
   if (defacl != (acl_t)NULL) {
     acl_free(defacl);
   }
-  if (fd >= 0 && close(fd) == -1) {
+  if (fd >= 0 && close(fd) == CLOSE_ERROR) {
     perror("apply_default_acl (close)");
     result = ACL_ERROR;
   }
@@ -948,11 +956,11 @@ bool apply_default_acl_recursive(const char *target, bool no_exec_mask) {
     return true;
   }
 
-  /* nftw will return -1 on error, or if the supplied function
+  /* nftw will return NFTW_ERROR on error, or if the supplied function
    * (apply_default_acl_nftw) returns a non-zero result, nftw will
    * return that.
    */
-  if (nftw_result == -1) {
+  if (nftw_result == NFTW_ERROR) {
     perror("apply_default_acl_recursive (nftw)");
   }
 
-- 
2.44.2