On Wed, 20 Oct 2021 19:02:30 GMT, Naoto Sato <na...@openjdk.org> wrote:

>> During the review of JEP 400, a proposal to provide an overloaded method to 
>> `Charset.forName()` was suggested 
>> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This 
>> PR is to implement the proposal. A CSR is also drafted as 
>> https://bugs.openjdk.java.net/browse/JDK-8275348
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Moved the null sentence into @param tag.

Oh, I found that I checked the outdated source code. Now this problem does not 
exist in `StringCoding`.

I simply browsed the latest source code of JDK and found that this idiom no 
longer appears outside jline. I believe that the source code of jline does not 
need to be modified in openjdk.

But I noticed `LauncherHelper.makePlatformString` 
(https://github.com/openjdk/jdk/blob/c978ca87de2d9152345dfd85983278c42bb28cd3/src/java.base/share/classes/sun/launcher/LauncherHelper.java#L887)

I don't understand why it stores `encoding` string and `isCharsetSupported` 
boolean values. Nor do I find references to these two fields in native code. I 
think it can benefit from the improvement in this PR like this?



    private static final String encprop = "sun.jnu.encoding";
    private static Charset charset = null;

    /*
     * converts a c or a byte array to a platform specific string,
     * previously implemented as a native method in the launcher.
     */
    static String makePlatformString(boolean printToStderr, byte[] inArray) {
        initOutput(printToStderr);
        if (charset == null) {
            charset = Charset.forName(encprop, Charset.defaultCharset());
        }
        return new String(inArray, charset);
    }

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

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

Reply via email to