RFR: 8303814: getLastErrorString should avoid charset conversions

[Naoto Sato]

On Wed, 8 Mar 2023 11:30: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.

Looks good (w/ some minor comments)

src/java.base/share/native/libzip/zip_util.c line 767:

> 765:  * or NULL if an error occurred. If a zip error occurred then *pmsg will
> 766:  * be set to the error message text if pmsg != 0. Otherwise, *pmsg will be
> 767:  * set to NULL. Caller doesn't need to free the error message.

I'd put some more context here why the caller does not need to free. (as it is a static text)

src/java.base/windows/native/libjava/jni_util_md.c line 80:

> 78:                 0,
> 79:                 buf,
> 80:                 sizeof(buf) / sizeof(WCHAR),

Maybe better to #define the size 256 so that this division is not needed.

src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 208:

> 206: 
> 207:     if (result == 0) {
> 208:         JNU_ThrowIOExceptionWithLastError(env, "Write failed");

Could be replaced with `JNU_ThrowIOException`?

src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 260:

> 258: 
> 259:     if (result == 0) {
> 260:         JNU_ThrowIOExceptionWithLastError(env, "Write failed");

Same as above

src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 299:

> 297: 
> 298:     if (result == 0) {
> 299:         JNU_ThrowIOExceptionWithLastError(env, "Write failed");

Same here

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

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