On Wed, 28 May 2025 15:25:40 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Several changes are made: >> >> 1. The "include" and "includedir" directives can appear everywhere, even >> inside a section or a sub-section. However, it only means the content is >> inserted here but the included file still need its own full structure -- >> from section to subsections. >> 2. The same file can be included multiple times as long as not recursively. >> 3. Everything is merged. For duplicated values, `get` returns the first one >> and `getAll` returns all joining by spaces. >> >> Two new tests added. I also separately confirmed that they are parsed in the >> same way as [MIT >> krb5](https://github.com/krb5/krb5/blob/master/src/util/profile/test_parse.c). >> MIT krb5 ignores directory name after "include" but here it's an error. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > more random testing src/java.security.jgss/share/classes/sun/security/krb5/Config.java line 608: > 606: // Already parsed. This is allowed. > 607: return; > 608: } Should this block of code be moved up before the line of `dups = new HashSet<>(dups);`? src/java.security.jgss/share/classes/sun/security/krb5/Config.java line 678: > 676: Path fullp = Paths.get(fileName).toAbsolutePath(); > 677: Path path = Paths.get(fileName); > 678: if (!Files.exists(path)) { Is `path` needed? Can't you just use `fullp` for the `!Files.exists()` check? src/java.security.jgss/share/classes/sun/security/krb5/Config.java line 818: > 816: * > 817: * @param entry path to config file, could be an included one > 818: * @returns the parsed configuration This method is changed to return void, why add this `@returns` ? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25421#discussion_r2153443893 PR Review Comment: https://git.openjdk.org/jdk/pull/25421#discussion_r2153220437 PR Review Comment: https://git.openjdk.org/jdk/pull/25421#discussion_r2153115117