On 05/08/2013 09:05, Alexey Utkin wrote:
Here is new version of the fix:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-7147084/webrev.02/
Changes (in accordance with Alan's recommendations):
-- Magic constant -1 was changed to JAVA_INVALID_HANDLE_VALUE with
explanation why the macro
INVALID_HANDLE_VALUE is not in use.
-- The comments were added for each usage of the
JAVA_INVALID_HANDLE_VALUE macro.
-- 10 seconds time limit was rewritten to file signals.
Regards,
-uta
Thanks for adding comments. I still think initHolder will be confusing
to future maintainers and could benefit from a clear comment (above the
static BOOL ...) to explain the parameters. One typo in the comment in "
[thisProcessEnd] has no the inherit flag.". The other functions are
mostly clear now (thanks).
There are a few formatting oddities in
Java_java_lang_ProcessImpl_create. Looks like a newline has crept into
the if statement at L337. Then L358-364 is mis-aligned which makes it
hard to tie the JNI ReleaseStringChars back to the JNI GetStringChars.
Thanks for replacing the fragile 10s in the tests. If I read performC
(in both tests) correctly then the test will still pass after 5 minutes
even if the JDK is broken - is that right? Just wonder if it would be
better to just replace the for loop with while (true) and let the test
harness timeout the test.
Otherwise I think this is good work and we are finally close to the
finish line.
-Alan.
