Review request: JDK-8016579 (process) IOException thrown by ProcessBuilder.start() method is incorrectly encoded (v.2)

[Martin Buchholz]

Looks OK to me.  As ever, we continue to not have good places within the
JDK itself for C-level infrastructure - win32Error is not specific to the
process api, and should really be pulled into some common directory - but I
don't know of any such.


On Tue, Jul 16, 2013 at 9:04 AM, Alexey Utkin <alexey.utkin at oracle.com>wrote:

>  Here is new version of fix:
> http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/
>
> On 7/15/2013 9:08 PM, 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.
>
> In java.lang we have passed the half-way: the error reporting sub-system
> is in the ASCII world.
>
>
>  Please spell correctly:
> MESAGE_LENGTH
>
> Thanks, I missed it. Fixed.
>
>
>  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?
>
> As Dan mentioned, the os_lasterror was a copy of JVM  os::lasterror call.
> The error message procedure is used for
>    CreatePipe
>    CreateProcessW
>    GetExitCodeProcess
>    WaitForMultipleObjects
> fail report. You are right, all the calls return the problem by
> GetLastMessage call.
> The function is changed (reduced).
>
> Here is new version of fix:
> http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8016579/webrev.01/
>
> Regards,
> -uta
>
>
>
>
> On Mon, Jul 15, 2013 at 2:41 AM, Alexey Utkin <alexey.utkin at 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/
>>
>> 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
>>
>
>
>