On Fri, 12 Jun 2026 18:38:26 GMT, Ashay Rane <[email protected]> wrote:

>> Although the original implementation of `JLI_Open()` borrowed from the
>> code in src/hotspot/os/windows/os_windows.cpp, the improvements to
>> os_windows.cpp don't seem to have been applied to java_md.c, causing
>> some tests to fail when the path to JAR files is longer than `MAXPATH`
>> (i.e. 260) characters on Windows (see associated JBS issue 8385024 for
>> details).  Since `JLI_Open()` is not just invoked inside tests, this is
>> not a test-specific issue, so fixing the test is not the right solution.
>> 
>> This patch applies the recent changes from os_windows.cpp to java_md.c
>> so that `JLI_Open()` can correctly handle longer than `MAXPATH` paths.
>> The new code is almost the same as that in `wide_abs_unc_path()` in
>> os_windows.cpp, except that the code in java_md.c uses `JLI_MemAlloc()`
>> and `JLI_MemFree()` for memory allocation and deallocation.
>> 
>> Although it would be ideal to have just one implementation between
>> HotSpot and the launcher, the dependencies of the two components
>> prevents us from having a single implementation.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Ashay Rane has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains 10 additional commits since 
> the last revision:
> 
>  - Merge branch 'microsoft:main' into JDK-8385024-jli-maxpath
>  - Do not rewrite paths beginning with \\.\ as UNC paths
>  - Add null check after `JLI_MemAlloc()`
>  - Address PR comments
>    
>    1. Dropped prefix argument since the lone caller passed an empty value.
>    
>    2. Added a block that I missed copying from `wide_abs_unc_path()` to
>       `convert_to_absolute_path()`.
>  - Address PR comments
>    
>    1. Use `Path.of(System.getProperty("user.dir"))` instead of
>       `Path.of(".").toAbsolutePath().normalize()` for clarity.
>    
>    2. Fix year in copyright notices.
>  - Remove statement that I had inserted for debugging
>  - Ensure hard failure when test doesn't pass
>    
>    Previously, when the `testLongPathJarFile()` or the
>    `testLongResolvedPathJarFile()` test cases failed, the code in
>    TestHelper would append an error message (that was checked at the end of
>    the test) but the test cases themselves didn't throw an exception, so it
>    wasn't clear _which_ test case failed.  More precisely, the output said:
>    
>    ```
>    Total: Passed: 7, Failed 0
>    Total of 2 failed
>    ```
>    
>    This patch updates these tests so that upon failure, they throw an
>    exception, causing the name of the failing test to be printed in the
>    error log.
>  - Add test to exercise the change
>    
>    This patch adds a new test so that the relative path (which becomes the
>    argument to `JLI_Open()`) is shorter than `MAX_PATH` but that the
>    absolute path (that is resolved inside `_open()`) is longer than
>    `MAX_PATH`.  To construct such a case, the test first probes the length
>    of the current working directory and then creates additional
>    subdirectories to meet the desired constraint.
>  - Address PR comments
>    
>    1. Optimize for the common case when the JAR path is shorter than
>       `MAX_PATH` by using a stack buffer of size length `MAX_PATH`.  If the
>       full path is longer than that, then we allocate a heap buffer of the
>       desired size before calling `GetFullPathNameW()` with a buffer of the
>       right size.
>    
>    2. Normalize the path before called `set_path_prefix()`, just like in
>       os_windows.cpp.  This ensures that prefixed paths with forward
>       slashes work correctly.
>    
>    3. Renamed `create_unc_path()` to `...

Marked as reviewed by alanb (Reviewer).

-------------

PR Review: https://git.openjdk.org/jdk/pull/31209#pullrequestreview-4514150559

Reply via email to