Review request: JDK-8016579 (process) IOException thrown by ProcessBuilder.start() method is incorrectly encoded (v.3)
alexey.utkin at oracle.com
Wed Jul 17 15:28:37 UTC 2013
Here is new version with Dan's correction:
On 7/16/2013 11:07 PM, Dan Xu wrote:
> I agree. Is jdk_util.c or jni_util.c or any file under native/common/
> a good place?
> The fix looks good to me.
> One small comment is to get the return value of swprintf() in
> win32Error(). If the return value is -1, an error situation needs to
> be handled even though it seems very rare. If it is not -1, then get
> the actual length of utf16_javaMessage. And pass the length of
> utf16_javaMessage to WideCharToMultiByte() instead of MESSAGE_LENGTH
> to make the code more efficient. Thanks!
> On 07/16/2013 10:49 AM, Martin Buchholz wrote:
>> 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 <mailto:alexey.utkin at oracle.com>> wrote:
>> Here is new version of fix:
>> 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:
>> 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
>> 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:
>>> On Mon, Jul 15, 2013 at 2:41 AM, Alexey Utkin
>>> <alexey.utkin at oracle.com <mailto:alexey.utkin at oracle.com>> wrote:
>>> Bug description:
>>> Here is the suggested fix:
>>> 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:
>>> 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
>>> size_t getLastErrorString(char *buf, size_t len)
>>> The bug contains the attachment
>>> that summarize the fix result in comparison with original
More information about the core-libs-dev