[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(m): 8140645: [aix] Recent Developments for AIX
From:       Thomas_Stüfe <thomas.stuefe () gmail ! com>
Date:       2015-10-30 11:58:11
Message-ID: CAA-vtUwTu8kWKvOm8BZQF7PLqPE55EiJK4FSrfNuOcgBu1f84Q () mail ! gmail ! com
[Download RAW message or body]

Hi Goetz,

thanks for reviewing!

Here the new version:
http://cr.openjdk.java.net/~stuefe/webrevs/8140645/webrev.01/webrev/index.html

Comments inline:

On Wed, Oct 28, 2015 at 5:46 PM, Lindenmaier, Goetz <
goetz.lindenmaier@sap.com> wrote:

> Hi Thomas,
>
> thanks for this cleanup, it's good to have the better version of
> loadlib in our port, too.  I'll sponsor this for you.
>
> If I understand correctly, you also moved some utilities from
> porting_aix.* to new
> files misc_aix.*.  That's fine. You missed the copyright message in
> misc_aix.cpp.
>
>
fixed.



> Please sort includes alphabetically.
>
>
done.


> Don't you want to move trcVerbose() to misc_aix, too?
>
>
done.


> Why do you choose buffer size 64 in debug builds in reload_table()?
> Maybe write "size_t buflen = 64 NOT_DEBUG(*16);"
>
>
Originally intent was to stress buffer resize code paths in debug builds,
but probably not needed anymore, you are right. I removed the
debug-specific size.


> Please fix guarantee0 not to call assert. This was wrong before,
> probably added by me in the very beginning.
>
>
Done, now maps to guarantee()


> You mention TODO in loadlib_aix.cpp about the double stringlists.
> As the other list (I assume you mean fixed_strings) is in aix coding
> too, why don't you consolidate both in misc_aix.hpp?
>
>
Good idea, though misc_aix.hpp would not be the right place. That file is
specifically for stubs to SAP infrastructure coding which we do not want
(yet) to completely port to the OpenJDK.

porting_aix.hpp would be the right place for this.

But I rather do this in a speparate change.


> I'll send you a patch with some formatting changes off-list. I think they
> fix some minor deviations from common coding style.  Feel free to
> apply them.
>
>
I applied them, thanks!

Best Regards, Thomas


> -----Original Message-----
> From: hotspot-runtime-dev [mailto:
> hotspot-runtime-dev-bounces@openjdk.java.net] On Behalf Of Thomas Stüfe
> Sent: Wednesday, October 28, 2015 12:09 PM
> To: hotspot-runtime-dev@openjdk.java.net
> Cc: ppc-aix-port-dev@openjdk.java.net
> Subject: RFR(m): 8140645: [aix] Recent Developments for AIX
>
> Hi,
>
> please review and sponsor these AIX-only changes. These changes bring a
> number of improvements to the AIX port which were originally developed at
> SAP.
>
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8140645/webrev.00/webrev/index.html
> https://bugs.openjdk.java.net/browse/JDK-8140645
>
> Thank you!
>
> Kind Regards, Thomas
>
[prev in list] [next in list] [prev in thread] [next in thread]