RFR: JDK-8227021: VM fails if any sun.boot.library.path paths are longer than JVM_MAXPATHLEN

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~afarley/8227021.2/webrev/src/hotspot/share/runtime/os.cpp.frames.html

This is code is unnecessarily hard to read and understand:

1329 char** os::split_path(const char* path, size_t* n) {
1330 size_t pathmod = (size_t)0;
1331 if (n != NULL) {
1332 pathmod = *n;
1333 }
1334 *n = (size_t)0;

It appears that 'n' is both an input and output parameter of this 
function.  Can you just make it another input parameter with some 
descriptive name.  Is it the length of the library name that you're 
looking for?  "pathmod" is huh?

Then you don't have to check if someone has passed NULL.  There's only 
one caller to this from what I see.

1352   char** opath = (char**) NEW_C_HEAP_ARRAY(char*, count, mtInternal);
1353   if (opath == NULL) {
1354     return NULL;
1355   }


Not your change but this calls vm_exit_out_of_memory() if 
NEW_C_HEAP_ARRAY fails.  It doesn't return NULL so you don't have to check.

1375     char* s  = (char*)NEW_C_HEAP_ARRAY(char, len + 1, mtInternal);

Here you want to use NEW_C_HEAP_ARRAY_RETURN_NULL so you can check for 
null and free memory.


The rest seems okay.

thanks,
Coleen

On 7/31/19 5:01 AM, Adam Farley8 wrote:
> Hi All,
>
> Reviewers requested for the change below.
>
> @David - Agreed. Would you be prepared to sponsor the change?
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8227021
> Webrev: http://cr.openjdk.java.net/~afarley/8227021.2/webrev/
>
> Best Regards
>
> Adam Farley
> IBM Runtimes
>
> P.S. Remembered to add the links this time. :)
>
>
> David Holmes <david.holmes at oracle.com> wrote on 30/07/2019 03:37:53:
>
>> From: David Holmes <david.holmes at oracle.com>
>> To: Adam Farley8 <adam.farley at uk.ibm.com>
>> Cc: hotspot-dev at openjdk.java.net, serviceability-dev
>> <serviceability-dev at openjdk.java.net>
>> Date: 30/07/2019 03:38
>> Subject: Re: RFR: JDK-8227021: VM fails if any sun.boot.library.path
>> paths are longer than JVM_MAXPATHLEN
>>
>> Hi Adam,
>>
>> On 25/07/2019 3:57 am, Adam Farley8 wrote:
>>> Hi David,
>>>
>>> Welcome back. :)
>> Thanks. Sorry for the delay in getting back to this.
>>
>> I like .v2 as it is much simpler (notwithstanding freeing the already
>> allocated arrays adds some complexity - thanks for fixing that).
>>
>> I'm still not sure we can't optimise things better for unchangeable
>> properties like the boot libary path, but that's another RFE.
>>
>> Thanks,
>> David
>>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>