river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonathan Costers <jonathan.cost...@googlemail.com>
Subject Re: svn commit: r999115 - in /incubator/river/jtsk/trunk/src: com/sun/jini/tool/classdepend/ net/jini/jeri/ net/jini/security/
Date Tue, 21 Sep 2010 09:22:51 GMT
Hi Peter

You are talking about this change, correct?


 /**
 * Permission required to dynamically grant permissions by security policy
@@ -554,7 +555,7 @@ public final class GrantPermission exten
     * of permissions.
     */
    private static String constructName(Permission[] pa) {
-       StringBuffer sb = new StringBuffer(60);
+       StringBuffer sb = new StringBuffer();


-> I'm curious, why is the above?


       for (int i = 0; i < pa.length; i++) {
           Permission p = pa[i];
           if (p instanceof UnresolvedPermission) {
@@ -675,7 +676,7 @@ public final class GrantPermission exten
    private static class Implier {

       private final PermissionCollection perms = new Permissions();
-       private final List unresolved =new ArrayList();
+       private final ArrayList unresolved = new ArrayList();


-> I'm curious, why is the above?


       void add(GrantPermission gp) {
           for (int i = 0; i < gp.grants.length; i++) {
@@ -752,8 +753,7 @@ public final class GrantPermission exten
     * @serial include
     */
    static class GrantPermissionCollection extends PermissionCollection {
-        // All access is synchronized through GrantPermissionCollection
-        // Nothing within should use synchronization
+
       private static final long serialVersionUID = 8227621799817733985L;

       /**
@@ -763,7 +763,7 @@ public final class GrantPermission exten
           new ObjectStreamField("perms", List.class, true)
       };

-       private List perms = new ArrayList(40);
+       private List perms = new ArrayList();


-> I'm curious, why is the above?


       private Implier implier = new Implier();

       public synchronized void add(Permission p) {
@@ -774,10 +774,8 @@ public final class GrantPermission exten
               throw new SecurityException(
                   "can't add to read-only PermissionCollection");
           }
-           if (!perms.contains(p)){
-               perms.add(p);
-               implier.add((GrantPermission) p);
-           }
+           perms.add(p);
+           implier.add((GrantPermission) p);
       }


-> I'm curious, why is the above?


Sorry again, I was quite tired last night ... This slipped my attention
while doing all these tiny changes to fix javadoc.

Best
Jonathan

2010/9/21 Jonathan Costers <jonathan.costers@googlemail.com>

> I will restore your improvements, apologies.
> What I wanted to do is back out some of your import removals, since they
> broke javadoc generation for those classes.
> Good to see people are awake :-)
>
> 2010/9/21 Peter Firmstone <jini@zeus.net.au>
>
> I'm curious about these changes, in your commit, what concerns me here is
>> not the removal of some performance improvements, but a bug that I found
>> while expanding UmbrellaGrant's.
>>
>> I guess I should have created a regression test and documented it.
>>
>> It relates to a memory out of bounds exception that is thrown if a dynamic
>> policy supports the use of UmbrellaGrantPermission, as requested on JIRA.
>>
>> Not everything I did should be considered experimental.  I've removed all
>> experimental changes.
>>
>> Rather than remove my hard work, wouldn't it be better to just find the
>> build that passes all tests in svn, like Patricia was doing?
>>
>> I'm assuming you removed it so a test passes?  If this change caused a
>> test failure, then the bug is not this piece of code, we should be
>> investigating why this change causes that test to fail, so we can solve the
>> bug, rather than obscure it.
>>
>> Peter.
>>
>>
>> jcosters@apache.org wrote:
>>
>>> Modified:
>>> incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java?rev=999115&r1=999114&r2=999115&view=diff
>>>
>>> ==============================================================================
>>> --- incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java
>>> (original)
>>> +++ incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java
>>> Mon Sep 20 20:56:19 2010
>>> @@ -41,6 +41,7 @@ import java.util.Enumeration;
>>>  import java.util.HashSet;
>>>  import java.util.Iterator;
>>>  import java.util.List;
>>> +import net.jini.security.policy.DynamicPolicy;
>>>  /**
>>>  * Permission required to dynamically grant permissions by security
>>> policy
>>> @@ -554,7 +555,7 @@ public final class GrantPermission exten
>>>      * of permissions.
>>>      */
>>>     private static String constructName(Permission[] pa) {
>>> -       StringBuffer sb = new StringBuffer(60);
>>> +       StringBuffer sb = new StringBuffer();
>>>        for (int i = 0; i < pa.length; i++) {
>>>            Permission p = pa[i];
>>>            if (p instanceof UnresolvedPermission) {
>>> @@ -675,7 +676,7 @@ public final class GrantPermission exten
>>>     private static class Implier {
>>>
>>>        private final PermissionCollection perms = new Permissions();
>>> -       private final List unresolved =new ArrayList();
>>> +       private final ArrayList unresolved = new ArrayList();
>>>        void add(GrantPermission gp) {
>>>            for (int i = 0; i < gp.grants.length; i++) {
>>> @@ -752,8 +753,7 @@ public final class GrantPermission exten
>>>      * @serial include
>>>      */
>>>     static class GrantPermissionCollection extends PermissionCollection {
>>> -        // All access is synchronized through GrantPermissionCollection
>>> -        // Nothing within should use synchronization +
>>>        private static final long serialVersionUID = 8227621799817733985L;
>>>        /**
>>> @@ -763,7 +763,7 @@ public final class GrantPermission exten
>>>            new ObjectStreamField("perms", List.class, true)
>>>        };
>>>  -      private List perms = new ArrayList(40);
>>> +       private List perms = new ArrayList();
>>>        private Implier implier = new Implier();
>>>        public synchronized void add(Permission p) {
>>> @@ -774,10 +774,8 @@ public final class GrantPermission exten
>>>                throw new SecurityException(
>>>                    "can't add to read-only PermissionCollection");
>>>            }
>>> -           if (!perms.contains(p)){
>>> -               perms.add(p);
>>> -               implier.add((GrantPermission) p);
>>> -           }
>>> +           perms.add(p);
>>> +           implier.add((GrantPermission) p);
>>>        }
>>>
>>>        public synchronized Enumeration elements() {
>>>
>>>
>>>
>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message