[7ux-osx] Please review: 7124089: launcher refactoring

[Anthony Petrov]

Hi Kumar,

src/share/bin/java.c
>  987         } else if (IsMacOSX() && JLI_StrCmp(arg, "-XstartOnFirstThread") == 0) {
>  988             ProcessSpecialArg(arg);
>  989         } else if (IsMacOSX() && JLI_StrCCmp(arg, "-Xdock:") == 0) {
>  990             ProcessSpecialArg(arg);

> 1595     if (IsMacOSX()) {

If the initial goal was to get rid of mentioning a platform name in the 
java.c, then "if (IsMacOSX())" instead of "#ifdef MACOSX" doesn't make 
sense as well. Also, what justifies replacing the compile time checks 
with run-time checks in these two occurrences? I realize that in both 
cases the code itself is completely correct, but I don't see why it 
should be even present/executed in the launcher for platforms other than 
the Mac.

Generally, the fix looks good. Lots of #ifdef MACOSX looked very 
confusing before. However, I feel uncomfortable with having so much code 
duplicated between src/solaris/bin/java_md.c and 
src/macosx/bin/java_md.c. This seems to increase launcher code 
maintenance cost. Is there any possibility to fold the code up in a 
common source file that is shared between solaris/linux and macosx, and 
only define really specific parts of the code in separate files? The 
simplest way to accomplish this would be to leave exactly one #ifdef 
MACOSX in the shared file and #include a platform specific part there. 
Or we could tweak the make files to compile an additional file.

Also, the major part of the JVMInit() function is identical on all three 
platforms - solaris/linux, macosx, and windows. It makes sense to define 
a function containing that code in the java.c and call it by 
platform-specific JVMInit() implementations where needed.

--
best regards,
Anthony

On 1/17/2012 7:33 AM, Kumar Srinivasan wrote:
> oops missed setting the subject line to 7ux-osx.
> 
> Kumar
> 
>> Hi,
>>
>> Please review the launcher refactoring work,  the webrev is here:
>> http://cr.openjdk.java.net/~ksrini/7124089/webrev.0/
>>
>> Notes:
>> 1. Moves the majority of the launcher code into platform specific 
>> directories/files,
>>     removed redundant conditionals.
>>
>> 2. The solaris/linux code co-exists together as before.
>>
>> 3. On MacOSX, "java -client" will launch the server VM, for 
>> compatibility reasons,
>>     and dual  mode is left in the macosx's java_md.c,  this will 
>> enable experiments
>>     with universal libraries,  quite easily. Also the mac ergonomics 
>> will always return
>>     server.
>>
>> Testing:
>>    Tested splash screen on all platforms and launcher regression tests.
>>
>> Thanks
>> Kumar
>>
>>
>>
>