On Wed, 4 Sep 2024 20:05:30 GMT, Simon Tooke <sto...@openjdk.org> wrote:
>> src/hotspot/os/windows/os_windows.cpp line 5344:
>>
>>> 5342: // In this case, use the user provided buffer but at least check
>>> whether _fullpath caused
>>> 5343: // a memory overwrite.
>>> 5344: if (errno == EINVAL) {
>>
>> There is nothing to indicate that `_fullpath` can ever set `EINVAL` it is
>> only specified to return null on error. This code should not check errno but
>> can just re-try with the user-supplied buffer.
>
> According to [the
> documentation](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fullpath-wfullpath?view=msvc-170),
> EINVAL can be set in some circumstances. Those circumstances are checked
> earlier in the function, so I have simplified this code.
>
> Setting EINVAL (or ENAMETOOLONG) is part of the existing documentation for
> os::realpath(); I am loathe to change it.
I missed the one case where it can return EINVAL (strange that is buried in the
remarks section).
Given that none of the callers of realpath ever even look at errno it seems
rather pointless to set it, but that cleanup should be a seperate RFE.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20683#discussion_r1745394594
