X-Git-Url: https://gitweb.michael.orlitzky.com/?a=blobdiff_plain;f=src%2Flibadacl.c;h=5bca89fe405dc1a030bdab0ef4ccd55b8d5eab71;hb=62844f5f2d280403511c4741226b39843955c08c;hp=a43afa5c1b203a63e7d905296a56719fa0a43d49;hpb=6b639194b7be7aa17e9736d1ed034e798d206de2;p=apply-default-acl.git diff --git a/src/libadacl.c b/src/libadacl.c index a43afa5..5bca89f 100644 --- a/src/libadacl.c +++ b/src/libadacl.c @@ -259,39 +259,47 @@ int acl_update_entry(acl_t aclp, acl_entry_t entry) { return ACL_ERROR; } + /* Our return value. Default to failure, and change to success if we + actually update something. */ + int result = ACL_FAILURE; + acl_entry_t existing_entry; /* Loop through the given ACL looking for matching entries. */ - int result = acl_get_entry(aclp, ACL_FIRST_ENTRY, &existing_entry); + int get_entry_result = acl_get_entry(aclp, ACL_FIRST_ENTRY, &existing_entry); - while (result == ACL_SUCCESS) { + while (get_entry_result == ACL_SUCCESS) { acl_tag_t existing_tag = ACL_UNDEFINED_TAG; if (acl_get_tag_type(existing_entry, &existing_tag) == ACL_ERROR) { perror("set_acl_tag_permset (acl_get_tag_type)"); - return ACL_ERROR; + result = ACL_ERROR; + goto cleanup; } if (existing_tag == entry_tag) { /* If we update something, we're done and return ACL_SUCCESS */ if (acl_set_permset(existing_entry, entry_permset) == ACL_ERROR) { - perror("acl_update_entry (acl_set_permset)"); - return ACL_ERROR; + perror("acl_update_entry (acl_set_permset)"); + result = ACL_ERROR; + goto cleanup; } - return ACL_SUCCESS; + result = ACL_SUCCESS; + goto cleanup; } - result = acl_get_entry(aclp, ACL_NEXT_ENTRY, &existing_entry); + get_entry_result = acl_get_entry(aclp, ACL_NEXT_ENTRY, &existing_entry); } /* This catches both the initial acl_get_entry and the ones at the end of the loop. */ - if (result == ACL_ERROR) { + if (get_entry_result == ACL_ERROR) { perror("acl_update_entry (acl_get_entry)"); - return ACL_ERROR; + result = ACL_ERROR; } - return ACL_FAILURE; + cleanup: + return result; } @@ -1004,21 +1012,29 @@ int apply_default_acl(const char* path, bool recursive) { } char* parent = dirname(dirname_path_copy); - /* Just kidding, if the path is ".", then dirname will do the wrong - * thing and give us "." as its parent, too. So, we handle that as a - * special case. - * - * WARNING: it is important that "parent" itself is not used after - * this point; otherwise we would need to store the correct parent - * path in there. But since everything uses file descriptors from - * now on, we only need to ensure that we get the correct parent_fd - * below. */ - if (strcmp(path, ".") == 0 && strcmp(parent, ".") == 0) { - parent_fd = safe_open("..", O_DIRECTORY | O_NOFOLLOW); + basename_path_copy = strdup(path); + if (basename_path_copy == NULL) { + perror("apply_default_acl (strdup)"); + result = ACL_ERROR; + goto cleanup; } - else { - parent_fd = safe_open(parent, O_DIRECTORY | O_NOFOLLOW); + char* child = basename(basename_path_copy); + + /* Just kidding, if the path is "." or "..", then dirname will do + * the wrong thing and give us "." as its parent, too. So, we handle + * those as special cases. We use "child" instead of "path" here to + * catch things like "./" and "../" + */ + bool path_is_dots = strcmp(child, ".") == 0 || strcmp(child, "..") == 0; + char dots_parent[6] = "../"; + if (path_is_dots) { + /* We know that "child" contains no more than two characters here, and + using strncat to enforce that belief keeps clang-tidy happy. */ + parent = strncat(dots_parent, child, 2); } + + parent_fd = safe_open(parent, O_DIRECTORY | O_NOFOLLOW); + if (parent_fd == OPEN_ERROR) { if (errno == ELOOP || errno == ENOTDIR) { /* We hit a symlink, either in the last path component (ELOOP) @@ -1034,24 +1050,19 @@ int apply_default_acl(const char* path, bool recursive) { } /* We already obtained the parent fd safely, so if we use the - basename of path here instead of the full thing, then we can get - away with using openat() and spare ourselves the slowness of - another safe_open(). */ - basename_path_copy = strdup(path); - if (basename_path_copy == NULL) { - perror("apply_default_acl (strdup)"); - result = ACL_ERROR; - goto cleanup; - } - - /* If the basename is ".", then we don't want to open "." relative - to the parent_fd, so we need another special case for that - path. */ - if (strcmp(path, ".") == 0 && strcmp(parent, ".") == 0) { - fd = open(".", O_NOFOLLOW); + * basename of path here instead of the full thing, then we can get + * away with using openat() and spare ourselves the slowness of + * another safe_open(). + * + * Note that if the basename is "." or "..", then we don't want to + * open it relative to the parent_fd, so we need another special + * case for those paths here. + */ + if (path_is_dots) { + fd = open(child, O_NOFOLLOW); } else { - fd = openat(parent_fd, basename(basename_path_copy), O_NOFOLLOW); + fd = openat(parent_fd, child, O_NOFOLLOW); } if (fd == OPEN_ERROR) { if (errno == ELOOP || errno == ENOTDIR) {