On Fri, 18 Aug 2023 14:22:49 GMT, Erik Joelsson <er...@openjdk.org> 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

Reply via email to