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 <[email protected]> wrote on 30/07/2019 03:37:53:

> From: David Holmes <[email protected]>

> To: Adam Farley8 <[email protected]>
> Cc: [email protected], serviceability-dev
> <[email protected]>

> 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


Reply via email to