[ 
https://issues.apache.org/jira/browse/HDFS-5673?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13852200#comment-13852200
 ] 

Haohui Mai commented on HDFS-5673:
----------------------------------

Acl:

Acl.Builder can initialize the entries field along with the declaration.
The constructor of Acl does not need to copy the list.

AclTransformation:

It seems to me that you can pass in a wrapper for Acl to indicate that the Acl 
list is well-formed. For example, you can have the following class:
 
{code}
class AclList {
  boolean wellFormed;
  List<AclEntry> acls;
}
{code}

and put Preconditions.check(AclList.wellFormed) in the functions of 
AclTransformation. Of course you might set the variable {{wellFormed}} 
incorrectly, but it is much easier to debug in the other places (and 
potentially formally verifying the logic as well).

I'm not sure that why AclTransformation needs to be a functor. It makes sense 
if you need to iterate through the ACL list for multiple times, otherwise 
making them as a function will be simpler and more efficient.

I think the code is probably overly optimized. For example, in 
filterAclEntriesByAclSpec(), I would probably write:

{code}
List<AclEntry> entries;
for (AclEntry existingEntry : existingAcl.getEntries()) {
  if (aclSpec.indexOf(existingEntry) != -1) {
    entries.add();
  } else {
    entries.delete();
  }
}
sort(entries);
return new Acl.Builder().setEntries(entries).build();
{code}

Obviously it is turning an O ( n ) algorithm to an O(nlogn) algorithm, but n is 
fairly small in this case (n < 32), and it is much simpler to read and to 
reason about. The code is security sensitive therefore I would favor of trading 
some efficiency to simplicity and readability.

> Implement logic for modification of ACLs.
> -----------------------------------------
>
>                 Key: HDFS-5673
>                 URL: https://issues.apache.org/jira/browse/HDFS-5673
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: namenode
>    Affects Versions: HDFS ACLs (HDFS-4685)
>            Reporter: Chris Nauroth
>            Assignee: Chris Nauroth
>         Attachments: HDFS-5673.1.patch
>
>
> This patch will include the core logic for modification of ACLs.  This 
> includes support for all user-facing APIs that modify ACLs.  This will cover 
> access ACLs, default ACLs, automatic mask calculations, automatic inference 
> of unprovided default ACL entries, and validation to prevent creation of an 
> invalid ACL.



--
This message was sent by Atlassian JIRA
(v6.1.4#6159)

Reply via email to