Hi Kumar,
Changes look good to me!
Thanks,
David
On 10/05/2017 5:45 AM, Kumar Srinivasan wrote:
Hi David,
I have made the following changes:
1. Removed the launcher's check for model flags, allowing the VM to
process
and fail, aligned the test ChangeDataModel to the VM's message.
2. Fixed up all the pre-existing conditions, pointed out earlier.
Full webrev:
http://cr.openjdk.java.net/~ksrini/8169646/webrev.1/
Incremental webrev (since last reviewed changeset):
http://cr.openjdk.java.net/~ksrini/8169646/webrev.1/webrev.delta/
Thanks
Kumar
Hi Kumar,
On 9/05/2017 12:27 AM, Kumar Srinivasan wrote:
Hi David,
Hi Kumar,
On 7/05/2017 1:21 AM, Kumar Srinivasan wrote:
Hello,
Please review the changes to remove the java launcher's data model
options -d32/-d64, these options existed to support Solaris dual-mode
operation, however for uniformity, these options were accepted by
other
platforms as well. These options are now obsolete, and deprecated in
jdk9,and are now being removed.
Webrev: http://cr.openjdk.java.net/~ksrini/8169646/webrev.0
Issue: https://bugs.openjdk.java.net/browse/JDK-8169646
This seems okay in itself, though I'm curious why you made the
launcher/jli responsible for treating -d32/-d64 as invalid options
instead of just passing them through to the VM to let it reject them
as it does any other non-existent option eg:
I mulled over this for sometime, and felt that since the launcher
supported these options before, it should give a better/reasonable
error message than the VM does. I am undecided. What do you think ?
I think all invalid options should be treated the same. We had the
deprecation phase for providing custom feedback - now we're just
rejecting them outright.
Cheers,
David
> java -foo -version
Unrecognized option: -foo
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.
I also see some pre-existing messiness you might want to tidy up while
you're in the area :)
OMG Pre-existing conditions. ;)
src/java.base/macosx/native/libjli/java_md_macosx.c
378 JLI_Snprintf(jvmcfg, so_jvmcfg, "%s%slib%s%s%sjvm.cfg",
379 jrepath, FILESEP, FILESEP, "", "");
The last two %s should just be removed instead of passing empty
strings.
422 } else {
423 /*
424 * macosx client library is built thin, i386 only.
425 * 64 bit client requests must load server library
426 */
427 const char *jvmtypeUsed = "server";
428 JLI_Snprintf(jvmpath, jvmpathsize, "%s/lib/%s/" JVM_DLL,
jrepath, jvmtypeUsed);
429 }
I don't think we have ever had 32-bit client on OSX! So this whole
clause seems strange. That aside you can just hardwire %s/lib/server
instead of using the jvmTypeUsed variable.
+1, I think we did support 32-bit for sometime, in the early days of
MacOSX.
---
src/java.base/unix/native/libjli/java_md_solinux.c
323 JLI_Snprintf(jvmcfg, so_jvmcfg, "%s%slib%s%sjvm.cfg",
324 jrepath, FILESEP, FILESEP, FILESEP);
Another strange version of this code - only 4 %s this time, but
adjacent FILESEP. :) I'm assuming this is from when <arch> was removed.
Urk.
Ok I will revisit the above, good catches!, and prepare
another changeset.
Kumar
Thanks,
David
Thanks
Kumar
PS: Thanks to David Holmes and Igor Ignatyev the -d32/-d64 flags have
been
purged from internal test suites.
