RFR: 8338851: Hoist os::Posix::realpath() to os::realpath() and implement on Windows [v2]

David Holmes dholmes at openjdk.org
Thu Sep 5 12:32:52 UTC 2024


On Wed, 4 Sep 2024 20:05:30 GMT, Simon Tooke <stooke at 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


More information about the serviceability-dev mailing list