7076310: (file) AclEntry.Builder setFlags throws IllegalArgumentException if set argument is empty

Stephen Flores stephen.flores at oracle.com
Wed Dec 21 12:56:11 PST 2011


On 12/21/2011 04:46 AM, Alan Bateman wrote:
> On 21/12/2011 05:51, Stephen Flores wrote:
>> Please review the following webrev (includes new test to demonstrate
>> the bug):
>>
>> http://cr.openjdk.java.net/~sflores/7076310/
> Thanks for taking one. It looks like the setPermissions method has the
> same issue so I think we should fix both together.
>

OK.

> The simplest solution is probably to replace EnumSet.copyOf(flags) with
> new HashSet<>(flags). What you have is okay too although in theory
> someone could clear the set at just around the time that setFlags is
> called in which case the copyOf method would see an empty set (as
> AclEntry.Builder is clearly specified to not be safe for use by multiple
> concurrent threads then your approach is okay).
>

If change setFlags(Set) and setPermissions(Set) to use a HashSet instead 
of a EnumSet, for consistency, should I also change 
setFlags(AclEntryFlags...) to use HashSet?

> The test should be added to test/java/nio/file/attribute/AclEntry.
> Looking at it now, it seems I neglected to include a unit test for
> AclEntry which is probably how this bug sipped through (it is tested by
> AclFileAttributeView as it tests reading and writing ACLs, just didn't a
> complete test for AclEntry).

It seems to me that jtreg expects one "main" class per unit test and I 
see this done for some packages, are you saying you want one "main" per 
class to be tested.

Steve.

>
> -Alan.


More information about the nio-dev mailing list