On Tue, 14 Oct 2025 21:53:41 GMT, Daniel Hu <[email protected]> wrote:
>> These changes should prevent entire binary files from being loaded into >> memory for build/AbsPathsInImage.java test. I chose a default buffer size of >> 8KB since BufferedInputStream uses that, but open to alternative solutions. >> GHA passes and test passes on linux x64. > > Daniel Hu has updated the pull request incrementally with two additional > commits since the last revision: > > - fix incorrect use of inputstream > - remove extraneous variables/imports I took a look at this again and I appreciate the effort you've put in. That said, the code is now quite convoluted and hard to understand. Would you mind adding some comments describing the algorithm on a high level and perhaps what each method is meant to accomplish? I would also like to know what verification of detection of failures you have done. If you deliberately allow absolute paths in the build, will the test properly detect them? The drawback of a complicated implementation is that it becomes hard to validate the test itself. I'm almost thinking we need a test for the test here, that you could invoke manually to make sure it detects positives. Specifically for corner cases when offending strings cross buffer boundaries. Will the new implementation handle a search pattern longer than the buffer size? I doubt we would ever need such a long pattern, but if not, perhaps the buffer size should be chosen dynamically to accommodate? ------------- PR Comment: https://git.openjdk.org/jdk/pull/26030#issuecomment-3427527173
