On Mon, 30 Mar 2026 08:51:48 GMT, Alan Bateman <[email protected]> wrote:
>> It's where it is because it's using the encapsulated FLAG_XXX constants. >> It's split out as this separate util function because the existing code has >> 2, completely separate locations, at which it needs the flags calculated. >> The docs for the method explain it: >> >> >> * <p>Since preview flags are calculated separately for resource nodes >> and >> * directory nodes (in two quite different places) it's useful to have a >> * common helper. >> >> >> If the existing code were re-worked to avoid processing resource and >> directory nodes in completely difference classes (`ImageResourcesTree` and >> `ImageFileCreator`), this could probably be neatened up. Any other solution >> would involve code duplication and/or breaking encapsulation. > > I would prefer if we could replace this with something simpler that doesn't > have a user provider predicate. Can you work through the usage in the jlink > image writer to see we could make t his simpler and easier to > understand/maintain. I already spent a long time trying to simplify it. I can spend more time on this, but I doubt it'll be much neater without a comprehensive refactoring of several classes. If you're happy with a such a change for this, I can try, but it will be quite disruptive to a lot of existing code. >> This is where I simply don't know enough about how the C++ API is used to >> know for sure if it could get such a value. If this is used for something >> like "Class.forName()" this it would be possible to construct something. > > Maybe we need to add a test that uses the 2-arg Class.forName with a Module > that is defined to the boot class loader to be sure. > > Update: I tried this: > > String className = "java.lang.String" + "X".repeat(5000); > Class.forName(Object.class.getModule(), className); > ``` > to cause JIMAGE_FindResource be called with a name > JIMAGE_MAX_PATH. So I > think the return 0L is right, not an assert as I initially thought. After testing there seems to be no way to have `/modules/` or `/packages/` as the leading part here (the C++ gets module names from existing modules. One thought I had (since we have control over the file format) is to just use `module` and `package` as the prefixes here, since those are conveniently already restricted keywords (at least in the Java language). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3065026169 PR Review Comment: https://git.openjdk.org/jdk/pull/29414#discussion_r3065014052
