Review request for 7129420: [macosx] SplashScreen.getSplashScreen() returns null

[Anthony Petrov]

Hi Kumar,

Thanks for refactoring the Java launcher code! I've just re-based my fix 
to the new structure of the Mac OS X launcher sources, here it is:

http://cr.openjdk.java.net/~anthony/x-8-getSplashScreenNull.1/

I've applied all the suggestions listed below. Thanks for the review!

Regarding your question about checking for dlopen() status - yes, we're 
OK if the splash screen library is unable to load. E.g. Embedded folks 
often remove some unnecessary parts from a JRE image to reduce its size. 
If they remove the splash screen dynamic library we'll just silently 
ignore the error and continue w/o showing the splash screen - not that 
it's much needed for embedded applications anyway.

The path lengths, however, are true error conditions, that's why I 
JLI_ReportErrorMessage() them with my fix.

--
best regards,
Anthony

On 1/20/2012 9:48 PM, Kumar Srinivasan wrote:
> Hi Anthony,
> 
> Do you want to wait until I push the launcher changes which
> are under review ?
> 
> In the launcher code we tend to use the JLI prefixed posix
> calls, ex: JLI_Snprintf. vs snprintf
> 
> The error messages are defined in emessages.h, this is so
> that we can L10N these messages in the future, if such a
> requirement arises. I think you can use JRE_ERROR11
> 
> Personally I would create two more buffers to avoid errors
> and make it safer, these are a cause of some grief when
> security audits are performed for buffer overruns.
> 
> char tail[PATH_MAX]
> char tmp[PATH_MAX]
> 
> JLI_Snprintf(tail, PATH_MAX, "/lib/%s", SPLASHSCREEN_SO);
> 
> if (JLI_Snprintf(tmp, PATH_MAX, "%s%s", path, tail)) {
>  JLI_ReportErrorMessage.......
> }
> 
> also I noticed that there is no check for dlopen, so is it ok for the
> splashscreen to fail silently if dlopen fails ?
> 
> Thanks
> Kumar
> 
> 
>> Hello,
>>
>> Please review a fix for http://bugs.sun.com/view_bug.do?bug_id=7129420 
>> at:
>>
>> http://cr.openjdk.java.net/~anthony/x-8-getSplashScreenNull.0/
>>
>> We remove our custom mechanism to determine the splash screen dynamic 
>> library path and instead start using the GetJREPath() for this 
>> purpose. I've verified this fix with an SDK (not JRE!) image, and it 
>> works just fine.
>>
>> -- 
>> best regards,
>> Anthony
>