On Mon, 2 May 2022 22:39:09 GMT, Mark Powers <d...@openjdk.java.net> wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8284688
>> 
>> [JDK-8273046](https://bugs.openjdk.java.net/browse/JDK-8273046) is the 
>> umbrella bug for this bug. The changes were too large for a single code 
>> review, so it was decided to split into smaller chunks. This is one such 
>> chunk: 
>> 
>> open/src/java.security.jgss/share/classes/javax/security 
>> open/src/java.security.jgss/share/classes/org/ietf 
>> open/src/java.security.jgss/share/classes/sun/security
>
> Mark Powers has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 11 commits:
> 
>  - sixth iteration
>  - Merge
>  - fifth iteration
>  - Merge
>  - fourth iteration
>  - third iteration
>  - Merge
>  - Merge
>  - Merge
>  - second iteration
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/64225e19...5ce46ffb

Many many thanks for this huge code change. No problem found. I have several 
comments and you're free to make your own decision.

src/java.security.jgss/share/classes/org/ietf/jgss/GSSException.java line 333:

> 331:     public String getMajorString() {
> 332: 
> 333:         return Objects.requireNonNullElseGet(majorString, () -> 
> messages[major - 1]);

Isn't this too fancy?

src/java.security.jgss/share/classes/sun/security/jgss/GSSHeader.java line 267:

> 265:                 throw new IOException("DerInputStream.getLength(): 
> lengthTag="
> 266:                                       + tmp + ", "
> 267:                                       + "too big.");

Combine the 2 lines above.

src/java.security.jgss/share/classes/sun/security/jgss/krb5/SubjectComber.java 
line 154:

> 152:                     while (iterator.hasNext()) {
> 153:                         Object obj = iterator.next();
> 154:                         if (!(obj instanceof 
> @SuppressWarnings("unchecked")KerberosTicket ticket)) {

Does not look fluent to me.

src/java.security.jgss/share/classes/sun/security/jgss/spnego/SpNegoContext.java
 line 876:

> 874: 
> 875:         // pass token
> 876:         tok = Objects.requireNonNullElseGet(token, () -> new byte[0]);

Is this how `requireNonNullElseGet` is meant to be used?

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

Marked as reviewed by weijun (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7746

Reply via email to