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

[David Holmes]

Hi Adam,

On 10/08/2019 2:47 am, Adam Farley8 wrote:
> Hi Serguei, David,
> 
> My turn to apologise for the delay. :)
> 
> I've modified the code as per Serguei's request. Take a look and let me 
> know if this is the sort of thing you were thinking of.
> 
> Webrev: http://cr.openjdk.java.net/~afarley/8227021.3/webrev/

I'd prefer to see the helper just as a file static function rather than 
adding it to the os class.

+  * supplied array of arrays of chars (a), where n

I assume (a) is meant to refer to the parameter, but you actually called 
it arrayarray. I think "a" or "arr" would suffice.

Thanks,
David

> Best Regards
> 
> Adam Farley
> IBM Runtimes
> 
> 
> "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote on 
> 31/07/2019 17:18:05:
> 
>> From: "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
>> To: Adam Farley8 <adam.farley at uk.ibm.com>,  David Holmes
>> <david.holmes at oracle.com>
>> Cc: serviceability-dev <serviceability-dev at openjdk.java.net>,
>> hotspot-dev at openjdk.java.net
>> Date: 31/07/2019 17:18
>> Subject: Re: RFR: JDK-8227021:  VM fails if any sun.boot.library.path
>> paths are longer than JVM_MAXPATHLEN
>> 
>> Hi Adam,
>> 
>> It looks Okay to me.
>> 
>> A couple of minor comments.
>> 
>> http://cr.openjdk.java.net/~afarley/8227021.2/webrev/src/hotspot/
>> share/runtime/os.cpp.frames.html
> 
>> 1362       //release  allocated storage before exiting the vm
>> 1363       while (i > 0) {
>> 1364           i--;
>> 1365           if (opath[i] != NULL) {
>> 1366             FREE_C_HEAP_ARRAY(char,  opath[i]);
>> 1367           }
>> 1368       }
>> 1369       FREE_C_HEAP_ARRAY(char*, opath);
>> 
>> 1377       //release allocated storage before returning  null
>> 1378       while (i > 0) {
>> 1379           i--;
>> 1380           if (opath[i] != NULL) {
>> 1381             FREE_C_HEAP_ARRAY(char,  opath[i]);
>> 1382           }
>> 1383       }
>> 1384       FREE_C_HEAP_ARRAY(char*, opath);
>> 
>> This duplicated fragments is worth to refactor to a function.
>> Also a space is missed at the beginning of the comment.
>> 
>> 
>> Thanks,
>> Serguei
>> 
>> 
>> 
>> On 7/31/19 02:01, 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
> 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