On Mon, 1 Jun 2026 20:48:48 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 incrementally with one additional 
> commit since the last revision:
> 
>   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.

Changes requested by macarte (Committer).

src/java.base/windows/native/libjli/java_md.c line 660:

> 658: 
> 659:     wchar_t* unicode_path = NULL;
> 660:     *err = convert_to_unicode(npath, L"", &unicode_path);

Even though it's not part of this PR, convert_to_unicode has a bug in the 
prefix part:

prefix_len = wcslen(prefix);
    wpath_len = prefix_len + unicode_path_len;
    *wpath = (wchar_t*)JLI_MemAlloc(wpath_len * sizeof(wchar_t));
    if (*wpath == NULL) {
        return ENOMEM;
    }

    wcsncpy(*wpath, prefix, prefix_len);
    if (MultiByteToWideChar(CP_ACP, MB_ERR_INVALID_CHARS,
                            path, -1, &((*wpath)[prefix_len]), (int)wpath_len) 
== 0) { <<<<< BUG
        JLI_MemFree(*wpath);
        *wpath = NULL;
        return EINVAL;
    }

unicode_path_len should be used instead of wpath_len

Although this method is now only called once and with an empty prefix, so the 
method could be simplifed (removing the bug)

src/java.base/windows/native/libjli/java_md.c line 688:

> 686:         *err = ENOMEM;
> 687:     } else {
> 688:         _snwprintf(result, result_len, L"%s%s", prefix, 
> &full_path[prefix_off]);

In os_windows.cpp there's another block I think we need:

// Remove trailing pathsep (not for \?<DRIVE>:, since it would make it relative)
    result_len = wcslen(result);
    if ((result[result_len - 1] == L'\') &&
        !(::iswalpha(result[4]) && result[5] == L':' && result_len == 7)) {
      result[result_len - 1] = L'\0';
    }

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

PR Review: https://git.openjdk.org/jdk/pull/31209#pullrequestreview-4422131882
PR Review Comment: https://git.openjdk.org/jdk/pull/31209#discussion_r3351429317
PR Review Comment: https://git.openjdk.org/jdk/pull/31209#discussion_r3351379825

Reply via email to