I guess Alexey made the changes based on the existing implementation of JVM_GetLastErrorString() in hotspot, which is os::lasterror(char* buf, size_t len) in os_windows.cpp.

One comment about the code at line 105 in win32Error().

 105     const int errnum = GetLastError();


Since in os_lasterror() function, both GetLastError() and errno are used conditionally to get the error code, will the same logic used here to get the value for errnum? Otherwise, it is possible that an error code and message are wrongly mapped and printed out. I need to point out that this issue is not brought up by this change, and also exists in the old code. Thanks!

-Dan


On 07/15/2013 10:08 AM, Martin Buchholz wrote:
Superficial review:

Looks good mostly.

Historically, switching windows code to use "W" APIs has been a big TODO, but was waiting for Win98 de-support.

Please spell correctly:
MESAGE_LENGTH

If errno and GetLastError are two separate error notification systems, how do you know which one corresponded to the last failure? E.g. if the last failure only set errno, won't the error message be via GetLastError(), which is likely to be stale?



On Mon, Jul 15, 2013 at 2:41 AM, Alexey Utkin <alexey.ut...@oracle.com <mailto:alexey.ut...@oracle.com>> wrote:

    Bug description:
    https://jbs.oracle.com/bugs/browse/JDK-8016579
    http://bugs.sun.com/view_bug.do?bug_id=8016579

    Here is the suggested fix:
    http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.00/
    <http://cr.openjdk.java.net/%7Euta/openjdk-webrevs/JDK-8016579/webrev.00/>

    Summary:
    We have THREE locales in action:
    1. Thread default locale - dictates UNICODE-to-8bit conversion
    2. OS locale that defines the message localization
    3. The file name locale

    Each locale could be an extended locale, that means that text
    cannot be mapped to 8bit sequence without multibyte encoding. VM
    is ready for that, if text is UTF-8.
    The suggested fix does the work right from the beginning.

    Unicode version of JVM call:
         hotspot/src/os/windows/vm/os_windows.cpp:
             size_t os::lasterror(char* buf, size_t len)
    was used as prototype for Unicode error message getter. It has to
    be fixed accordingly as well as
         jdk/src/windows/native/java/io/io_util_md.c
             size_t getLastErrorString(char *buf, size_t len)

    The bug contains the attachment
    https://jbs.oracle.com/bugs/secure/attachment/14581/JDK-8016579.txt
    that summarize the fix result in comparison with original
    implementation.

    Regards,
    -uta



Reply via email to