RFR(S): 8203354: assert in ClassLoader::update_module_path_entry_list() could have incorrect message

[Calvin Cheung]

Hi Thomas,

Thanks for your quick review.

On 5/22/18, 1:36 PM, Thomas Stüfe wrote:
> Hi Calvin,
>
> some small nits:
>
> - could you use vm_exit_during_initialization instead of raw exit?
> - "Bad path %s" - that is an assumption, stat() may fail for a number
> of reasons. So I would make the message more neutral. I also always
> enclose such an output in quotes, to catch if the caller had
> leading/trailing spaces. So, maybe this: "CDS dump aborted (path was
> \"%s\")." ?
I've fixed the above.
Updated webrev: http://cr.openjdk.java.net/~ccheung/8203354/webrev.01/
>
> Side note: I looked at the various os::stat implementations and see a
> number of issues. We copy the input path around, which is on Posix
> platforms unnecessary since os::native_path() returns just the path;
> and since we use a fixed sized buffer we artificially limit the path
> os::stat() recognizes. That is bad. Also, on windows, we return the
> Win32 error code (GetLastError) as errno, which is just plain wrong.
>
> But these are issues beyond your patch.
Could you file bugs on the above issues?

thanks,
Calvin
>
> Kind Regards, Thomas
>
>
> On Tue, May 22, 2018 at 9:08 PM, Calvin Cheung<calvin.cheung at oracle.com>  wrote:
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8203354
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/8203354/webrev.00/
>>
>> Converting the assert in ClassLoader::update_module_path_entry_list() to a
>> meaningful error message before aborting the CDS dump.
>>
>> With a module path with very long length (>  2K), the error message will be
>> like the following:
>>
>> os::stat error 36 (ENAMETOOLONG). CDS dump aborted. Bad path
>> /scratch/myDir/JTwork/scratch/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx<snip>
>>
>> Note that tty->print_cr() has a buffer size limit of 2000, so the full path
>> won't be shown entirely. That's why I'd like to put the error code up front
>> before the path.
>>
>> Ran the modified tests on Oracle platforms.
>>
>> thanks,
>> Calvin