]> arthur.barton.de Git - netatalk.git/commitdiff
More error checking
authorFrank Lahm <franklahm@googlemail.com>
Tue, 28 Sep 2010 10:06:06 +0000 (12:06 +0200)
committerFrank Lahm <franklahm@googlemail.com>
Tue, 28 Sep 2010 10:06:06 +0000 (12:06 +0200)
doc/DEVELOPER
etc/afpd/acls.c
include/atalk/errchk.h

index a2678fd409eec9d2213432395011d2de400e8113..9ba1765bcf4fa9364f8be79be7f9349beb84a89b 100644 (file)
@@ -164,10 +164,16 @@ checking and logging statements.
 In order to alleviate error checking and code readability, we provide a set
 of error checking macros in <atalk/errchk.h>. These macros compare the return
 value of statements againt 0, NULL, -1 (and maybe more, check it out).
-Every macro comes in two flavours: EC_CHECK and EC_CHECK_LOG, the difference
-being that the *LOG macro logs the error.
-Both macros unconditionally jump to a cleanup label where the neccessary
-cleanup can be done alongside controlling the return value.
+Every macro comes in four flavours: EC_CHECK, EC_CHECK_LOG, EC_CHECK_LOG_ERR
+and EC_CHECK_CUSTOM:
+- EC_CHECK just checks the CHECK
+- EC_CHECK_LOG additionally logs the stringified function call.
+- EC_CHECK_LOG_ERR allows specifying the return value
+- EC_CHECK_CUSTOM allows custom actions
+The macros EC_CHECK* unconditionally jump to a cleanup label where the
+neccessary cleanup can be done alongside controlling the return value.
+EC_CHECK_CUSTOM doesn't do that, so an extra "goto EC_CLEANUP" may be
+performed as appropiate.
 
 Example:
 - stat() without EC macro:
@@ -191,12 +197,16 @@ Example:
   static int func(const char *name) {
     EC_INIT; /* expands to int ret = 0; */
 
-    EC_ZERO(stat(name, &some_struct_stat)); /* expands to complete if block from above */
+    char *uppername = NULL
+    EC_NULL(uppername = strdup(name));
+    EC_ZERO(strtoupper(uppername));
 
-    return 0; /* ret is hidden, so return explicit success value */
+    EC_ZERO(stat(uppername, &some_struct_stat)); /* expands to complete if block from above */
+
+    EC_STATUS(0);
 
 EC_CLEANUP:
-    ...
+    if (uppername) free(uppername);
     EC_EXIT;
   }
 
@@ -208,6 +218,8 @@ int func(void)
 
     ...your code here...
 
+    EC_STATUS(0);
+
 EC_CLEANUP:
     EC_EXIT;
 }
index d65f5c08f77d7140587197fad63609b1b35a70b5..4a7e1449f3abbf2ea0d7b2dff53f0c0ed04fd2e9 100644 (file)
@@ -648,12 +648,13 @@ static int map_acl(int type, const void *acl, darwin_ace_t *buf, int ace_count)
    Returns 0 on success, -1 on error. */
 static int get_and_map_acl(char *name, char *rbuf, size_t *rbuflen)
 {
-    int ace_count = 0;
+    EC_INIT;
     int mapped_aces = 0;
-    int err, dirflag;
+    int dirflag;
     uint32_t *darwin_ace_count = (u_int32_t *)rbuf;
 #ifdef HAVE_SOLARIS_ACLS
-    ace_t *aces;
+    int ace_count = 0;
+    ace_t *aces = NULL;
 #endif
 #ifdef HAVE_POSIX_ACLS
     struct stat st;
@@ -666,71 +667,47 @@ static int get_and_map_acl(char *name, char *rbuf, size_t *rbuflen)
     rbuf += 4;
 
 #ifdef HAVE_SOLARIS_ACLS
-    if ( (ace_count = get_nfsv4_acl(name, &aces)) == -1) {
-        LOG(log_error, logtype_afpd, "get_and_map_acl: couldnt get ACL");
-        return -1;
-    }
-
-    if ( (mapped_aces = map_acl(SOLARIS_2_DARWIN, aces, (darwin_ace_t *)rbuf, ace_count)) == -1) {
-        err = -1;
-        goto cleanup;
-    }
+    EC_NEG1(ace_count = get_nfsv4_acl(name, &aces));
+    EC_NEG1(mapped_aces = map_acl(SOLARIS_2_DARWIN, aces, (darwin_ace_t *)rbuf, ace_count));
 #endif /* HAVE_SOLARIS_ACLS */
 
 #ifdef HAVE_POSIX_ACLS
     acl_t defacl = NULL , accacl = NULL;
 
     /* stat to check if its a dir */
-    if (stat(name, &st) != 0) {
-        LOG(log_error, logtype_afpd, "get_and_map_acl: stat: %s", strerror(errno));
-        err = -1;
-        goto cleanup;
-    }
+    EC_ZERO_LOG(stat(name, &st));
 
     /* if its a dir, check for default acl too */
     dirflag = 0;
     if (S_ISDIR(st.st_mode)) {
         dirflag = IS_DIR;
-        if ((defacl = acl_get_file(name, ACL_TYPE_DEFAULT)) == NULL) {
-            LOG(log_error, logtype_afpd, "get_and_map_acl: couldnt get default ACL");
-            err = -1;
-            goto cleanup;
-        }        
-        if ((mapped_aces = map_acl(POSIX_DEFAULT_2_DARWIN | dirflag,
-                                   defacl,
-                                   (darwin_ace_t *)rbuf,
-                                   0)) == -1) {
-            err = -1;
-            goto cleanup;
-        }
+        EC_NULL_LOG(defacl = acl_get_file(name, ACL_TYPE_DEFAULT));
+        EC_NEG1(mapped_aces = map_acl(POSIX_DEFAULT_2_DARWIN | dirflag,
+                                      defacl,
+                                      (darwin_ace_t *)rbuf,
+                                      0));
     }
 
-    if ((accacl = acl_get_file(name, ACL_TYPE_ACCESS)) == NULL) {
-        LOG(log_error, logtype_afpd, "get_and_map_acl: couldnt get access ACL");
-        err = -1;
-        goto cleanup;
-    }
+    EC_NULL_LOG(accacl = acl_get_file(name, ACL_TYPE_ACCESS));
 
     int tmp;
-    if ((tmp = map_acl(POSIX_ACCESS_2_DARWIN | dirflag,
-                      accacl,
-                      (darwin_ace_t *)(rbuf + mapped_aces * sizeof(darwin_ace_t)),
-                       0)) == -1) {
-        err = -1;
-        goto cleanup;
-    }
+    EC_NEG1(tmp = map_acl(POSIX_ACCESS_2_DARWIN | dirflag,
+                          accacl,
+                          (darwin_ace_t *)(rbuf + mapped_aces * sizeof(darwin_ace_t)),
+                          0));
     mapped_aces += tmp;
 #endif /* HAVE_POSIX_ACLS */
 
     LOG(log_debug, logtype_afpd, "get_and_map_acl: mapped %d ACEs", mapped_aces);
 
-    err = 0;
     *darwin_ace_count = htonl(mapped_aces);
     *rbuflen += sizeof(darwin_acl_header_t) + (mapped_aces * sizeof(darwin_ace_t));
 
-cleanup:
+    EC_STATUS(0);
+
+EC_CLEANUP:
 #ifdef HAVE_SOLARIS_ACLS
-    free(aces);
+    if (aces) free(aces);
 #endif
 #ifdef HAVE_POSIX_ACLS
     if (defacl) acl_free(defacl);
@@ -738,7 +715,8 @@ cleanup:
 #endif /* HAVE_POSIX_ACLS */
 
     LOG(log_debug9, logtype_afpd, "get_and_map_acl: END");
-    return err;
+
+    EC_EXIT;
 }
 
 /* Removes all non-trivial ACLs from object. Returns full AFPERR code. */
@@ -767,7 +745,8 @@ static int remove_acl(const struct vol *vol,const char *path, int dir)
 #ifdef HAVE_SOLARIS_ACLS
 static int set_acl(const struct vol *vol, char *name, int inherit, char *ibuf)
 {
-    int ret, i, nfsv4_ace_count;
+    EC_INIT;
+    int i, nfsv4_ace_count;
     int tocopy_aces_count = 0, new_aces_count = 0, trivial_ace_count = 0;
     ace_t *old_aces, *new_aces = NULL;
     uint16_t flags;
@@ -795,11 +774,10 @@ static int set_acl(const struct vol *vol, char *name, int inherit, char *ibuf)
     }
 
     /* Now malloc buffer exactly sized to fit all new ACEs */
-    new_aces = malloc( (ace_count + tocopy_aces_count) * sizeof(ace_t) );
-    if (new_aces == NULL) {
+    if (EC_NULL_CUSTOM(new_aces = malloc((ace_count + tocopy_aces_count) * sizeof(ace_t)))) {
         LOG(log_error, logtype_afpd, "set_acl: malloc %s", strerror(errno));
-        ret = AFPERR_MISC;
-        goto cleanup;
+        EC_STATUS(AFPERR_MISC);
+        goto EC_CLEANUP;
     }
 
     /* Start building new ACL */
@@ -817,10 +795,12 @@ static int set_acl(const struct vol *vol, char *name, int inherit, char *ibuf)
     LOG(log_debug7, logtype_afpd, "set_acl: copied %d inherited ACEs", new_aces_count);
 
     /* Now the ACEs from the client */
-    ret = map_acl(DARWIN_2_SOLARIS, &new_aces[new_aces_count], (darwin_ace_t *)ibuf, ace_count);
-    if (ret == -1) {
-        ret = AFPERR_PARAM;
-        goto cleanup;
+    if (EC_NEG1_CUSTOM(map_acl(DARWIN_2_SOLARIS,
+                               &new_aces[new_aces_count],
+                               (darwin_ace_t *)ibuf,
+                               ace_count))) {
+        EC_STATUS(AFPERR_PARAM);
+        goto EC_CLEANUP;
     }
     new_aces_count += ace_count;
     LOG(log_debug7, logtype_afpd, "set_acl: mapped %d ACEs from client", ace_count);
@@ -838,42 +818,43 @@ static int set_acl(const struct vol *vol, char *name, int inherit, char *ibuf)
     /* Ressourcefork first.
        Note: for dirs we set the same ACL on the .AppleDouble/.Parent _file_. This
        might be strange for ACE_DELETE_CHILD and for inheritance flags. */
-    if ( (ret = vol->vfs->vfs_acl(vol, name, ACE_SETACL, new_aces_count, new_aces)) != 0) {
+    if (EC_ZERO_CUSTOM(vol->vfs->vfs_acl(vol, name, ACE_SETACL, new_aces_count, new_aces))) {
         LOG(log_error, logtype_afpd, "set_acl: error setting acl: %s", strerror(errno));
         if (errno == (EACCES | EPERM))
-            ret = AFPERR_ACCESS;
+            EC_STATUS(AFPERR_ACCESS);
         else if (errno == ENOENT)
-            ret = AFPERR_NOITEM;
+            EC_STATUS(AFPERR_NOITEM);
         else
-            ret = AFPERR_MISC;
-        goto cleanup;
+            EC_STATUS(AFPERR_MISC);
+        goto EC_CLEANUP;
     }
-    if ( (ret = acl(name, ACE_SETACL, new_aces_count, new_aces)) != 0) {
+    if (EC_ZERO_CUSTOM(acl(name, ACE_SETACL, new_aces_count, new_aces))) {
         LOG(log_error, logtype_afpd, "set_acl: error setting acl: %s", strerror(errno));
         if (errno == (EACCES | EPERM))
-            ret = AFPERR_ACCESS;
+            EC_STATUS(AFPERR_ACCESS);
         else if (errno == ENOENT)
-            ret = AFPERR_NOITEM;
+            EC_STATUS(AFPERR_NOITEM);
         else
-            ret = AFPERR_MISC;
-        goto cleanup;
+            EC_STATUS(AFPERR_MISC);
+        goto EC_CLEANUP;
     }
 
-    ret = AFP_OK;
+    EC_STATUS(AFP_OK);
 
-cleanup:
-    free(old_aces);
-    free(new_aces);
+EC_CLEANUP:
+    if (old_aces) free(old_aces);
+    if (new_aces) free(new_aces);
 
     LOG(log_debug9, logtype_afpd, "set_acl: END");
-    return ret;
+    EC_EXIT;
 }
 #endif /* HAVE_SOLARIS_ACLS */
 
 #ifdef HAVE_POSIX_ACLS
 static int set_acl(const struct vol *vol, char *name, int inherit, char *ibuf)
 {
-    int ret = AFP_OK;
+    EC_INIT;
+    int is_dir = 0;
 
     LOG(log_maxdebug, logtype_afpd, "set_acl: BEGIN");
 
@@ -885,39 +866,24 @@ static int set_acl(const struct vol *vol, char *name, int inherit, char *ibuf)
     if (inherit) {
         /* get default ACEs */
         struct stat st;
-        if (stat(name, &st) != 0) {
-            LOG(log_error, logtype_afpd, "set_acl: stat: %s", strerror(errno));
-            ret = AFPERR_NOOBJ;
-            goto exit;
-        }
+        EC_ZERO_LOG_ERR(stat(name, &st), AFPERR_NOOBJ);
         if (S_ISDIR(st.st_mode)) {
-            if ((acl = acl_get_file(name, ACL_TYPE_DEFAULT)) == NULL) {
-                LOG(log_error, logtype_afpd, "set_acl: acl_get_file error: %s",
-                    strerror(errno));
-                ret = AFPERR_MISC;
-                goto exit;
-            }
+            is_dir = 1;
+            EC_NULL_LOG_ERR(acl = acl_get_file(name, ACL_TYPE_DEFAULT), AFPERR_MISC);
         }
+    } else {
+        EC_NULL_LOG_ERR(acl = acl_init(0), AFPERR_MISC);
     }
 
-    /*  */
-    if (acl == NULL && (acl = acl_init(0)) == NULL) {
-        LOG(log_error, logtype_afpd, "set_acl: acl_get_file error: %s",
-            strerror(errno));
-        ret = AFPERR_MISC;
-        goto exit;
-    }
-    /* acl now can take the additional ACEs as requested by the client */
+    EC_ZERO_LOG(map_acl_darwin_to_posix((darwin_ace_t *)ibuf, &acl, is_dir, ace_count));
 
-    while (ace_count) {
-        
-    }
+    EC_STATUS(AFP_OK);
 
-exit:
+EC_CLEANUP:
     if (acl)
         acl_free(acl);
     LOG(log_maxdebug, logtype_afpd, "set_acl: END: %u", ret);
-    return ret;
+    EC_EXIT;
 }
 #endif /* HAVE_POSIX_ACLS */
 
index af603841fcfbff17f2525d77deb0b40a2283c879..0ebadb5091d8ee67da94aeccb8fe9cc75cac40c0 100644 (file)
 #define ERRCHECK_H
 
 #define EC_INIT int ret = 0
+#define EC_STATUS(a) ret = (a)
 #define EC_CLEANUP cleanup
 #define EC_EXIT return ret
 
 /* 
+ * Check out doc/DEVELOPER for more infos.
+ *
  * We have these macros:
- * EC_ZERO_LOG
- * EC_ZERO
- * EC_NEG1_LOG
- * EC_NEG1
- * EC_NULL_LOG
- * EC_NULL
+ * EC_ZERO, EC_ZERO_LOG, EC_ZERO_LOG_ERR, EC_ZERO_CUSTOM
+ * EC_NEG1, EC_NEG1_LOG, EC_NEG1_LOG_ERR, EC_NEG1_CUSTOM
+ * EC_NULL, EC_NULL_LOG, EC_NULL_LOG_ERR, EC_NULL_CUSTOM
+ *
+ * A boileplate function template is:
+
+   int func(void)
+   {
+       EC_INIT;
+
+       ...your code here...
+
+       EC_STATUS(0);
+
+   EC_CLEANUP:
+       EC_EXIT;
+   }
  */
 
 /* check for return val 0 which is ok, every other is an error, prints errno */
-#define EC_ZERO_LOG(a) \
-    do { \
-        if ((a) != 0) { \
+#define EC_ZERO_LOG(a)                          \
+    do {                                        \
+        if ((a) != 0) {                                                 \
             LOG(log_error, logtype_default, "%s failed: %s" #a, strerror(errno)); \
-            ret = -1; \
-            goto cleanup; \
+            ret = -1;                                                   \
+            goto cleanup;                                               \
         } \
     } while (0)
 
+#define EC_ZERO_LOG_ERR(a, b)                   \
+    do { \
+        if ((a) != 0) {                                                 \
+            LOG(log_error, logtype_default, "%s failed: %s" #a, strerror(errno)); \
+            ret = (b);                                                  \
+            goto cleanup;                                               \
+        }                                                               \
+    } while (0)
+
 /* check for return val 0 which is ok, every other is an error */
 #define EC_ZERO(a) \
     do { \
         } \
     } while (0)
 
+/* check for return val 0 which is ok, every other is an error */
+#define EC_ZERO_CUSTOM(a) \
+    (ret = (a)) != 0
+
 /* check for return val 0 which is ok, every other is an error, prints errno */
 #define EC_NEG1_LOG(a) \
     do { \
         } \
     } while (0)
 
+#define EC_NEG1_LOG_ERR(a, b)                   \
+    do {                                        \
+        if ((a) == -1) {                                                \
+            LOG(log_error, logtype_default, "%s failed: %s" #a, strerror(errno)); \
+            ret = b;                                                    \
+            goto cleanup;                                               \
+        }                                                               \
+    } while (0)
+
 /* check for return val 0 which is ok, every other is an error */
 #define EC_NEG1(a) \
     do { \
         } \
     } while (0)
 
+#define EC_NEG1_CUSTOM(a) \
+    (ret = (a)) == -1
+
 /* check for return val != NULL, prints errno */
 #define EC_NULL_LOG(a) \
     do { \
         } \
     } while (0)
 
+#define EC_NULL_LOG_ERR(a, b)                   \
+    do {                                        \
+        if ((a) == NULL) {                                              \
+            LOG(log_error, logtype_default, "%s failed: %s" #a, strerror(errno)); \
+            ret = b;                                                    \
+            goto cleanup;                                               \
+        }                                                               \
+    } while (0)
+
 /* check for return val != NULL */
 #define EC_NULL(a) \
     do { \
         } \
     } while (0)
 
+#define EC_NULL_CUSTOM(a) \
+    (ret = (a)) == NULL
+
 #endif /* ERRCHECK_H */