RFR: 8303814: getLastErrorString should avoid charset conversions [v3]

Julian Waters jwaters at openjdk.org
Tue Mar 14 17:50:01 UTC 2023


On Mon, 13 Mar 2023 15:55:27 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> This patch modifies the `getLastErrorString` method to return a `jstring`. Thanks to that we can avoid unnecessary back and forth conversions between Unicode and other charsets on Windows.
>> 
>> Other changes include:
>> - the Windows implementation of `getLastErrorString` no longer checks `errno`. I verified all uses of the method and confirmed that `errno` is not used anywhere. 
>> - While at it, I found and fixed a few calls to `JNU_ThrowIOExceptionWithLastError` that were done in context where `LastError` was not set.
>> - jdk.hotspot.agent was modified to use `JNU_ThrowByNameWithLastError` and `JNU_ThrowByName` instead of `getLastErrorString`; the code is expected to have identical behavior.
>> - zip_util was modified to return static messages instead of generated ones. The generated messages were not observed anywhere, because they were replaced by a static message in ZIP_Open, which is the only method used by other native code.
>> - `getLastErrorString` is no longer exported by libjava.
>> 
>> Tier1-3 tests continue to pass.
>> 
>> No new automated regression test; testing this requires installing a language pack that cannot be displayed in the current code page.
>> Tested this manually by installing Chinese language pack on English Windows 11, selecting Chinese language, then checking if the message on exception thrown by `InetAddress.getByName("nonexistent.local");` starts with `"不知道这样的主机。"` (or `"\u4e0d\u77e5\u9053\u8fd9\u6837\u7684\u4e3b\u673a\u3002"`). Without the change, the exception message started with a row of question marks.
>
> Daniel Jeliński has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use NULL where appropriate

The change in Windows behaviour seems like a worrying gotcha that someone using the method might be trapped by (C library errors and system errors are reported for Unix while only WINAPI is returned on Windows). Although this has already been pushed I'd still like to mention that me and Thomas did have quite a bit of discussion regarding the conundrum on Windows about whether to report both when the error routines are called or have separate methods/mechanism to select either, see [8292016](https://bugs.openjdk.org/browse/JDK-8292016?filter=-1)

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

PR: https://git.openjdk.org/jdk/pull/12922


More information about the core-libs-dev mailing list