On Fri, 18 Aug 2023 14:22:49 GMT, Erik Joelsson <[email protected]> wrote:
> In the JDK build we have various build tools that generate source code from
> data files. For most of these tools, the source files are based on template
> files, which already have copyright headers, but for some, the complete
> source file is generated by the tool, which is providing the copyright header
> programatically. For the latter, we would like to implement an override
> mechanism in each tool so that we can change the copyright header from a
> custom makefile.
Seems reasonable.
One suggested change to make the tool command-line processing more robust, but
up to you whether to take it or not.
Thanks.
make/jdk/src/classes/build/tools/generatelsrequivmaps/EquivMapsGenerator.java
line 62:
> 60: StandardCharsets.UTF_8);
> 61: i++;
> 62: } else if (args.length != 3) {
This is not very robust for detecting malformed invocations. It won't even
detect a simple typo in the argument name. I would suggest something more
elaborate e.g.
int i 0;
boolean valid = true;
if (args.length != 5 && args.length !=3) {
valid = false;
} else if (args.length == 5) {
if ( ! "-jdk-header-template".equals(args[i])) {
valid = false;
} else {
jdkHeaderTemplate = new String(Files.readAllBytes(Paths.get(args[++i])),
StandardCharsets.UTF_8);
i++;
}
}
if (!valid) {
System.err.println(...);
System.exit(1);
}
String lsrFile = args[i++];
...
-------------
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15346#pullrequestreview-1592770524
PR Review Comment: https://git.openjdk.org/jdk/pull/15346#discussion_r1303797947
