On Wed, 30 Oct 2024 23:23:11 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request incrementally with 20 >> additional commits since the last revision: >> >> - Some test fixes >> - Remove period in jlink.properties >> - Revert changes to ResourcePoolEntry >> - Fix comment in RuntimeImageLinkException >> - Remove ImageReader (like JmodsReader) >> - More comment fixes (JlinkTask) >> - Move some comments around >> - More comment fix-ups (JRTArchive) >> - Fix description of configure option >> - Some more wording updates >> - ... and 10 more: https://git.openjdk.org/jdk/compare/83a86d06...e6b3aeb1 > > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java > line 399: > >> 397: String mResource = String.format(DIFF_PATH, module); >> 398: List<ResourceDiff> diff = perModDiffs.get(module); >> 399: if (diff == null) { > > Can `diff` ever be null? ` preparePerModuleDiffs` ensures that the return > map has one entry for each module and empty list if no diff. > > For the case with no diff, do you expect EMPTY_RESOURCE_BYTES or a > `ResourceDiff` content with header & zero entry? > > Perhaps line 377-386 in `preparePerModuleDiffs` can simply be removed and let > `addDiffResourcesFiles` to handle no resource diff case as it does now. > > > Map<String, List<ResourceDiff>> allModsToDiff = new HashMap<>(); > modules.stream().forEach(m -> { > List<ResourceDiff> d = modToDiff.get(m); > if (d == null) { > // Not all modules will have a diff > allModsToDiff.put(m, Collections.emptyList()); > } else { > allModsToDiff.put(m, d); > } > }); > return allModsToDiff; The diff can never be null. `ReasourceDiff.read(InputStream)` expects at least a proper header. So for a module without diff there will be small files with two integers, the magic and `0` (number of items). I've changed this code to no longer check for `null` and expanded the comment. Thanks! ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14787#discussion_r1828178083
