RFR: 8323624: ProviderList.ServiceList does not need to be a list [v4]

Sean Mullan mullan at openjdk.org
Thu Mar 28 15:13:46 UTC 2024


On Tue, 5 Mar 2024 14:55:06 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Re-implement it as an `Iterator` to make sure it can only be iterated once and make debugger happy.
>> 
>> No regression, just a refactoring.
>
> Weijun Wang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Merge branch 'master' into 8323624
>  - rename class name
>  - use consistent names
>  - : iterator
>  - the fix

Also add a `noreg` label to issue.

src/java.base/share/classes/sun/security/jca/GetInstance.java line 112:

> 110:      * necessary.
> 111:      */
> 112:     public static List<Service> getServices(String type, String algorithm) {

Change the javadoc of these methods from "Return a List of all ..." to "Returns an iterator over all ..."

src/java.base/share/classes/sun/security/jca/GetInstance.java line 123:

> 121:      */
> 122:     @Deprecated
> 123:     public static Iterator<Service> getServices(String type,

Can we remove this deprecated method yet?

src/java.base/share/classes/sun/security/jca/ProviderList.java line 402:

> 400:      * The List returned is NOT thread safe.
> 401:      */
> 402:     public Iterator<Service> getServices(String type, String algorithm) {

Change the javadoc of this method from "Return a List of all ..." to "Returns an iterator over all ..." and replace other instances of "List" with "Iterator".

src/java.base/share/classes/sun/security/jca/ProviderList.java line 412:

> 410:      */
> 411:     @Deprecated
> 412:     public Iterator<Service> getServices(String type, List<String> algorithms) {

Can we remove this deprecated method yet?

src/java.base/share/classes/sun/security/jca/ProviderList.java line 429:

> 427:      * Not thread safe.
> 428:      */
> 429:     private final class ServiceList extends AbstractList<Service> {

Replace comments above of instances of "List" with "Iterator".

src/java.base/share/classes/sun/security/jca/ProviderList.java line 539:

> 537:         int index;
> 538: 
> 539:         public boolean hasNext() {

Suggest adding Override annotations to overridden methods.

src/java.base/share/classes/sun/security/jca/ProviderList.java line 565:

> 563:          * ServiceList.
> 564:          */
> 565:         ArrayList<PreferredEntry> getAll(ServiceIterator s) {

line 563: s/ServiceList/ServiceIterator/

-------------

PR Review: https://git.openjdk.org/jdk/pull/17381#pullrequestreview-1966465307
PR Review Comment: https://git.openjdk.org/jdk/pull/17381#discussion_r1543125796
PR Review Comment: https://git.openjdk.org/jdk/pull/17381#discussion_r1543126321
PR Review Comment: https://git.openjdk.org/jdk/pull/17381#discussion_r1543131268
PR Review Comment: https://git.openjdk.org/jdk/pull/17381#discussion_r1543135914
PR Review Comment: https://git.openjdk.org/jdk/pull/17381#discussion_r1543137500
PR Review Comment: https://git.openjdk.org/jdk/pull/17381#discussion_r1543139173
PR Review Comment: https://git.openjdk.org/jdk/pull/17381#discussion_r1543141780



More information about the security-dev mailing list