You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@river.apache.org by jc...@apache.org on 2010/09/20 22:56:20 UTC

svn commit: r999115 - in /incubator/river/jtsk/trunk/src: com/sun/jini/tool/classdepend/ net/jini/jeri/ net/jini/security/

Author: jcosters
Date: Mon Sep 20 20:56:19 2010
New Revision: 999115

URL: http://svn.apache.org/viewvc?rev=999115&view=rev
Log:
RIVER-351: fix all javadoc generation warnings for the main project

Modified:
    incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDepend.java
    incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependParameters.java
    incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependencyRelationship.java
    incubator/river/jtsk/trunk/src/net/jini/jeri/BasicJeriExporter.java
    incubator/river/jtsk/trunk/src/net/jini/security/GrantPermission.java
    incubator/river/jtsk/trunk/src/net/jini/security/ProxyPreparer.java

Modified: incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDepend.java
URL: http://svn.apache.org/viewvc/incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDepend.java?rev=999115&r1=999114&r2=999115&view=diff
==============================================================================
--- incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDepend.java (original)
+++ incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDepend.java Mon Sep 20 20:56:19 2010
@@ -312,7 +312,8 @@ public class ClassDepend {
      * original API of ClassDep.
      * @param dependencyRelationShipMap The initial map before filtering.
      * @param cdp The parameters for filtration.
-     * @see ClassDependParameters, ClassDependencyRelationship
+     * @see ClassDependParameters
+     * @see ClassDependencyRelationship
      * @return Set<ClassDependencyRelationShip> result The result after filtration.
      */
     public Set filterClassDependencyRelationShipMap(Map dependencyRelationShipMap, ClassDependParameters cdp){
@@ -475,7 +476,7 @@ public class ClassDepend {
      * '.*' match classes in the package, names that end in '.**' match classes
      * in the package and it's subpackage.  Other names match the class.
      * @param names
-     * @return 
+     * @return Pattern
      */
     public Pattern createPattern(Collection names) {
 	if (names.isEmpty()) {

Modified: incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependParameters.java
URL: http://svn.apache.org/viewvc/incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependParameters.java?rev=999115&r1=999114&r2=999115&view=diff
==============================================================================
--- incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependParameters.java (original)
+++ incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependParameters.java Mon Sep 20 20:56:19 2010
@@ -30,7 +30,8 @@ import java.util.List;
  * return a ClassDependParamters object instance.
  * 
  * @author Peter Firmstone
- * @see ClassDepend, CDPBuilder
+ * @see ClassDepend
+ * @see CDPBuilder
  */
 public class ClassDependParameters {
     /* outsidePackagesOrClasses excluded from search ,excludes the names of classes,
@@ -143,8 +144,8 @@ public class ClassDependParameters {
          * root directory, to decend recursively into and exclude subpackages, 
          * the package pattern should end in .**
          * 
-         * 
-         * @see ClassDepend, ClassDependParameters
+         * @see ClassDepend
+         * @see ClassDependParameters
          * @return CDPBuilder so named optional parameters can be chained
          */
         public CDPBuilder addOutsidePackageOrClass(String outsidePackageOrClass) {
@@ -243,7 +244,9 @@ public class ClassDependParameters {
          * from the dependency search.
          * If false the platform classes returned will depend on the Java
          * platform and version the test is executing on.
-         * @see ClassDepend, ClassDependParameters
+         * 
+         * @see ClassDepend
+         * @see ClassDependParameters
          * @param b
          * @return CDPBuilder - enables optional parameter method chaining.
          */

Modified: incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependencyRelationship.java
URL: http://svn.apache.org/viewvc/incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependencyRelationship.java?rev=999115&r1=999114&r2=999115&view=diff
==============================================================================
--- incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependencyRelationship.java (original)
+++ incubator/river/jtsk/trunk/src/com/sun/jini/tool/classdepend/ClassDependencyRelationship.java Mon Sep 20 20:56:19 2010
@@ -25,7 +25,6 @@ import java.util.Set;
 /**
  * A container to store class dependency related information for later analysis.
  * @author Peter Firmstone
- * @Threadsafe
  * @see ClassDepend
  */
 public class ClassDependencyRelationship {
@@ -92,7 +91,7 @@ public class ClassDependencyRelationship
 
     /**
      * Get the classes that this class needs to function.
-     * @return
+     * @return a Set of classes
      */
     public Set getProviders() {
         Set prov = new HashSet();

Modified: incubator/river/jtsk/trunk/src/net/jini/jeri/BasicJeriExporter.java
URL: http://svn.apache.org/viewvc/incubator/river/jtsk/trunk/src/net/jini/jeri/BasicJeriExporter.java?rev=999115&r1=999114&r2=999115&view=diff
==============================================================================
--- incubator/river/jtsk/trunk/src/net/jini/jeri/BasicJeriExporter.java (original)
+++ incubator/river/jtsk/trunk/src/net/jini/jeri/BasicJeriExporter.java Mon Sep 20 20:56:19 2010
@@ -19,14 +19,26 @@
 package net.jini.jeri;
 
 import com.sun.jini.jeri.internal.runtime.BasicExportTable;
+import com.sun.jini.logging.Levels;
 import java.lang.ref.WeakReference;
 import java.rmi.Remote;
+import java.rmi.RemoteException;
 import java.rmi.server.ExportException;
+import java.rmi.server.Unreferenced;
+import java.security.AccessControlContext;
+import java.security.PrivilegedAction;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import net.jini.config.Configuration;
 import net.jini.export.Exporter;
+import net.jini.export.ServerContext;
 import net.jini.id.Uuid;
 import net.jini.id.UuidFactory;
+import net.jini.io.MarshalInputStream;
+import net.jini.io.context.ClientHost;
+import net.jini.io.context.ClientSubject;
+import net.jini.security.Security;
+import net.jini.security.SecurityContext;
 
 /**
  * An <code>Exporter</code> implementation for exporting
@@ -672,7 +684,6 @@ public final class BasicJeriExporter imp
      * 
      * @return the string representation for this exporter
      **/
-    @Override
     public String toString() {
 	return "BasicJeriExporter[" + se + "," + id + "]";
     }

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() {

Modified: incubator/river/jtsk/trunk/src/net/jini/security/ProxyPreparer.java
URL: http://svn.apache.org/viewvc/incubator/river/jtsk/trunk/src/net/jini/security/ProxyPreparer.java?rev=999115&r1=999114&r2=999115&view=diff
==============================================================================
--- incubator/river/jtsk/trunk/src/net/jini/security/ProxyPreparer.java (original)
+++ incubator/river/jtsk/trunk/src/net/jini/security/ProxyPreparer.java Mon Sep 20 20:56:19 2010
@@ -19,6 +19,9 @@
 package net.jini.security;
 
 import java.rmi.RemoteException;
+import net.jini.config.Configuration;
+import net.jini.core.constraint.RemoteMethodControl;
+import net.jini.security.Security;
 
 /**
  * Performs operations on a newly unmarshalled remote proxy to prepare it for



Re: svn commit: r999115 - in /incubator/river/jtsk/trunk/src: com/sun/jini/tool/classdepend/ net/jini/jeri/ net/jini/security/

Posted by Peter Firmstone <ji...@zeus.net.au>.
Jonathan Costers wrote:
> Hi Peter
>
> You are talking about this change, correct?
>   

Yes.
>
>  /**
>  * 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();
>   

16 characters wasn't enough to avoid resizing.

>
> -> 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?
>   

Can't remember, there shouldn't be too many unresolved permissions, so 
I've haven't set a default size greater than 16, I've probably changed 
the ArrayList to List, simply because List is a standard interface and 
no ArrayList specific methods were used.  Minor Refactoring.

>
>        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?
>   

This prevents duplicates being added to perms, under some circumstances 
a Memory exception occurred while expanding UmbrellaGrantPermission 
dynamic grants, in the DynamicPolicy (ConcurrentDynamicPolicyProvider), 
DynamicPolicyProvider doesn't expand UmbrellaGrantPermission's.  Feature 
was requested on Jira.

I'd have to go over the code again to remember exactly how it was 
caused, but this was part of the fix, it was something to do with 
expanding UmbrellaGrant's getting caught in an ever expanding loop.

I added a comment about synchronization also.

Peter.

>
> 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 <jo...@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 <ji...@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() {
>>>>
>>>>
>>>>
>>>>         
>>>       
>
>   


Re: svn commit: r999115 - in /incubator/river/jtsk/trunk/src: com/sun/jini/tool/classdepend/ net/jini/jeri/ net/jini/security/

Posted by Jonathan Costers <jo...@googlemail.com>.
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 <jo...@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 <ji...@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() {
>>>
>>>
>>>
>>
>>
>

Re: svn commit: r999115 - in /incubator/river/jtsk/trunk/src: com/sun/jini/tool/classdepend/ net/jini/jeri/ net/jini/security/

Posted by Jonathan Costers <jo...@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 <ji...@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() {
>>
>>
>>
>
>

Re: svn commit: r999115 - in /incubator/river/jtsk/trunk/src: com/sun/jini/tool/classdepend/ net/jini/jeri/ net/jini/security/

Posted by Peter Firmstone <ji...@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() {
>
>