On Fri, 1 Nov 2024 21:32:23 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Comments to clarify, also align skipOverFieldSignature
>
> src/java.base/share/classes/jdk/internal/constant/ConstantUtils.java line 258:
>
>> 256: if (name.isEmpty())
>> 257: return name;
>> 258: return validateBinaryClassName(name);
>
> Perhaps have a utility method to specify if empty name is allowed.
> `validateBinaryClassName` and `validateInternalClassName` only differ in the
> separator charactor. They can be refactored to call one single name
> validation method something like this:
>
>
> private static String validateName(String name, boolean internalName,
> boolean allowEmptyName) {
> if (!allowEmptyName && name.isEmpty())
> throw invalidClassName(name);
>
> // state variable for detection of illegal states, such as:
> // empty unqualified name, consecutive, leading, or trailing
> separators
> int afterSeparator = 0;
> int len = name.length();
> for (int i = 0; i < len; i++) {
> char ch = name.charAt(i);
> // reject ';' or '[' or other form's separator
> if (ch == ';' || ch == '[' || (internalName && ch == '.') ||
> (!internalName && ch == '/'))
> throw invalidClassName(name);
> if ((internalName && ch == '/') || (!internalName && ch == '.')) {
> // illegal state when received separator indicates consecutive
> // or leading separators
> if (i == afterSeparator)
> throw invalidClassName(name);
> afterSeparator = i + 1;
> }
> }
> // reject empty unqualified name or trailing separators
> if (len == afterSeparator)
> throw invalidClassName(name);
> return name;
> }
I assume I will still keep the 4 public methods to call this implementation
method, as the boolean arguments will be otherwise confusing. Is that what you
intend?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21830#discussion_r1826385667
