X-Git-Url: https://gitweb.michael.orlitzky.com/?a=blobdiff_plain;ds=sidebyside;f=src%2Fapply-default-acl.c;h=4a6ef765e10379d213661dacf39d8a08636cb20c;hb=6d396c52c731f260cc261e26c75034bd5007f0b0;hp=a3c28fb59ca963db23f2dcf79150a320cf40c947;hpb=927c66a6c66bcbd69b62f07f4b937c5df9afab88;p=apply-default-acl.git

diff --git a/src/apply-default-acl.c b/src/apply-default-acl.c
index a3c28fb..4a6ef76 100644
--- a/src/apply-default-acl.c
+++ b/src/apply-default-acl.c
@@ -158,7 +158,7 @@ bool path_accessible(const char* path) {
  *
  * @return true if @c path is a directory, false otherwise.
  */
-bool is_directory(const char* path) {
+bool is_path_directory(const char* path) {
   if (path == NULL) {
     return false;
   }
@@ -269,7 +269,13 @@ int acl_set_entry(acl_t* aclp,
      ACL. */
   acl_entry_t new_entry;
 
-  /* We allocate memory here that we should release! */
+  /* The acl_create_entry() function can allocate new memory and/or
+   * change the location of the ACL structure entirely. When that
+   * happens, the value pointed to by aclp is updated, which means
+   * that a new acl_t gets "passed out" to our caller, eventually to
+   * be fed to acl_free(). In other words, we should still be freeing
+   * the right thing, even if the value pointed to by aclp changes.
+   */
   int c_result = acl_create_entry(aclp, &new_entry);
   if (c_result == ACL_ERROR) {
     perror("acl_set_entry (acl_create_entry)");
@@ -312,20 +318,20 @@ int acl_set_entry(acl_t* aclp,
  * @brief Determine the number of entries in the given ACL.
  *
  * @param acl
- *   A pointer to an @c acl_t structure.
+ *   The ACL to inspect.
  *
  * @return Either the non-negative number of entries in @c acl, or
  *   @c ACL_ERROR on error.
  */
-int acl_entry_count(acl_t* acl) {
+int acl_entry_count(acl_t acl) {
 
   acl_entry_t entry;
   int entry_count = 0;
-  int result = acl_get_entry(*acl, ACL_FIRST_ENTRY, &entry);
+  int result = acl_get_entry(acl, ACL_FIRST_ENTRY, &entry);
 
   while (result == ACL_SUCCESS) {
     entry_count++;
-    result = acl_get_entry(*acl, ACL_NEXT_ENTRY, &entry);
+    result = acl_get_entry(acl, ACL_NEXT_ENTRY, &entry);
   }
 
   if (result == ACL_ERROR) {
@@ -344,14 +350,14 @@ int acl_entry_count(acl_t* acl) {
  * An ACL is minimal if it has fewer than four entries.
  *
  * @param acl
- *   A pointer to an acl_t structure.
+ *   The ACL whose minimality is in question.
  *
  * @return
  *   - @c ACL_SUCCESS - @c acl is minimal
  *   - @c ACL_FAILURE - @c acl is not minimal
  *   - @c ACL_ERROR - Unexpected library error
  */
-int acl_is_minimal(acl_t* acl) {
+int acl_is_minimal(acl_t acl) {
 
   int ec = acl_entry_count(acl);
 
@@ -450,7 +456,7 @@ int acl_execute_masked(acl_t acl) {
  */
 int any_can_execute_or_dir(const char* path) {
 
-  if (is_directory(path)) {
+  if (is_path_directory(path)) {
     /* That was easy... */
     return ACL_SUCCESS;
   }
@@ -465,7 +471,7 @@ int any_can_execute_or_dir(const char* path) {
   /* Our return value. */
   int result = ACL_FAILURE;
 
-  if (acl_is_minimal(&acl)) {
+  if (acl_is_minimal(acl)) {
     mode_t mode = get_mode(path);
     if (mode & (S_IXUSR | S_IXOTH | S_IXGRP)) {
       result = ACL_SUCCESS;
@@ -563,7 +569,7 @@ int assign_default_acl(const char* path, acl_t acl) {
     return ACL_ERROR;
   }
 
-  if (!is_directory(path)) {
+  if (!is_path_directory(path)) {
     return ACL_FAILURE;
   }
 
@@ -702,7 +708,7 @@ int apply_default_acl(const char* path, bool no_exec_mask) {
     return ACL_FAILURE;
   }
 
-  if (!is_regular_file(path) && !is_directory(path)) {
+  if (!is_regular_file(path) && !is_path_directory(path)) {
     return ACL_FAILURE;
   }
 
@@ -712,7 +718,7 @@ int apply_default_acl(const char* path, bool no_exec_mask) {
   path_copy[PATH_MAX-1] = 0;
 
   char* parent = dirname(path_copy);
-  if (!is_directory(parent)) {
+  if (!is_path_directory(parent)) {
     /* Make sure dirname() did what we think it did. */
     return ACL_FAILURE;
   }
@@ -755,7 +761,8 @@ int apply_default_acl(const char* path, bool no_exec_mask) {
   acl_t acl = acl_get_file(path, ACL_TYPE_ACCESS);
   if (acl == (acl_t)NULL) {
     perror("apply_default_acl (acl_get_file)");
-    return ACL_ERROR;
+    result = ACL_ERROR;
+    goto cleanup;
   }
 
   /* If it's a directory, inherit the parent's default. */
@@ -815,7 +822,22 @@ int apply_default_acl(const char* path, bool no_exec_mask) {
       }
     }
 
-    /* Finally, add the permset to the access ACL. */
+    /* Finally, add the permset to the access ACL. It's actually
+     * important that we pass in the address of "acl" here, and not
+     * "acl" itself. Why? The call to acl_create_entry() within
+     * acl_set_entry() can allocate new memory for the entry.
+     * Sometimes that can be done in-place, in which case everything
+     * is cool and the new memory gets released when we call
+     * acl_free(acl).
+     *
+     * But occasionally, the whole ACL structure will have to be moved
+     * in order to allocate the extra space. When that happens,
+     * acl_create_entry() modifies the pointer it was passed (in this
+     * case, &acl) to point to the new location. We want to call
+     * acl_free() on the new location, and since acl_free() gets
+     * called right here, we need acl_create_entry() to update the
+     * value of "acl". To do that, it needs the address of "acl".
+     */
     int set_result = acl_set_entry(&acl, entry);
     if (set_result == ACL_ERROR) {
       perror("apply_default_acl (acl_set_entry)");
@@ -945,7 +967,7 @@ int apply_default_acl_nftw_x(const char *target,
  */
 bool apply_default_acl_recursive(const char *target, bool no_exec_mask) {
 
-  if (!is_directory(target)) {
+  if (!is_path_directory(target)) {
     return apply_default_acl(target, no_exec_mask);
   }