RFE 4519026: (process) Process should support Unicode on Win NT, request for review (original) (raw)

Heiko Wagner heiko.wagner at apis.de
Fri Mar 20 10:33:38 UTC 2009

Thanks for your reply. I have updated both the source file and the diff file. (URL see in previous post below) I have made the following changes:

I have tried to set up jtreg and run the regression tests. Somehow, I still haven't managed to get all things working. Some tests in ProcessBuilder/Basic.java fail, because the exit code of the invocation of the java child is 6 insted of 0. I am still working on that issue.

P.S.: Is there any kind of guide line how to write the comments, so I can fix them as well?

-----Original Message----- From: Martin Buchholz [mailto:martinrb at google.com] Sent: Dienstag, 17. März 2009 02:32 To: Heiko Wagner Cc: core-libs-dev at openjdk.java.net; Xueming Shen; Alan Bateman Subject: Re: RFE 4519026: (process) Process should support Unicode on Win NT, request for review

Sorry for the delayed response. I've been busy. (Probably I should not have volunteered for this review.)

Heiko, thanks for the patch.

JDK engineers (Xueming, Alan?) will need to help with testing, architectural issues, and shepherding. I no longer use windows.

I approve of the general approach being taken here. We need a general purpose version of JVM_NativePath, as you have coded it up, but it needs to go into some shared location for use by other JDK native code - not sure quite where that would be. I guess until we have another client for it, living in ProcessImpl_md.c is not so bad.

Make sure you run the java.lang regression tests, especially ProcessBuilder/Basic.java

There's a fair bit of cleanup that will be required. Use of white space and commenting style are non-standard and will need to be fixed (even if you've copied them from other parts of the JDK)

I think the spec for free() guarantees that free(null) is a no-op, and the JDK already relies on this, so no need for releaseStringCopy.

Typos/suggestions:

path.md.c => path_md.c

*(res + len) = 0; => res[len] = L'\0';

Martin

On Mon, Mar 9, 2009 at 02:57, Heiko Wagner <heiko.wagner at apis.de> wrote:

This is related to my previous post at http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-February/001145.ht ml and my first contribution to the JDK7 project. As Martin suggested, I have worked on a wide char version of ProcessImplmd.c. For discussion of my proposed chages a copy of my source can be found at:

http://www.apis.de/pub/jdk7/ProcessImplmd.c and a diff at: http://www.apis.de/pub/jdk7/ProcessImplmd.c.diff This patch enables launching executables residing on a path containing non-ansi characters. My next goal is to get the java launcher working on a unicode path. I think this needs additional coordination with the hotspot team, since some of the code in os.cpp also has issues in a unicode path when loading verify.dll and java.dll. Regards Heiko

More information about the core-libs-dev mailing list