RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

[Alan Bateman]

On 20/11/2013 18:26, Volker Simonis wrote:
> Hi,
>
> this is the second review round for "8024854: Basic changes and files 
> to build the class library on AIX 
> <https://bugs.openjdk.java.net/browse/JDK-8024854>". The previous 
> reviews can be found at the end of this mail in the references section.
>
> I've tried to address all the comments and suggestions from the first 
> round and to further streamline the patch (it perfectly builds on 
> Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). 
> The biggest change compared to the first review round is the new 
> "aix/" subdirectory which I've now created under "jdk/src" and which 
> contains AIX-only code.
Thanks for the update and addressing all the original comments and 
suggestions. In particular, moving most of the AIX specific files to 
src/aix and including an implementation of dladdr, make a big difference 
and makes this much easier to review.

I've skimmed through all the non-client files in the webrev and just 
have a few comments:

NetworkLibraries.gmk - is the exclude of bsd_close.c right? It looks 
like it will add this to LIBNET_EXCLUDE_FILES even when building on Mac.

In the old verifier code (check_code.c) then it's not clear to me that 
the caller wrapper is needed but in any case the change suggests to me 
that we should look at the malloc usages so that they better handle the 
size==0 case. I realize the wrappers are to avoid changing too much and 
it should be okay to handle this via a separate bug.

In net_util.c then it's a bit ugly to be calling aix_close_init. 
Michael/Chris - what you would think about the JNI_OnLoad calling into a 
platform specific function to do platform specific initialization?

The changes to java_md_solinux.c look okay to me but it makes me wonder 
if this should be renamed as it no longer exclusively Solaris + Linux.

Port.java - one suggestion for unregisterImpl is to rename it to 
preUnregister and change it to protected so that it's more obvious that 
it supposed to be overridden.

UnixNativeDispatcher.c - this looks okay (must reduced since the first 
round), I just wonder if the changes to *_getpwuid and *_getgrgid are 
really needed as this just impacts the error message. Also might be good 
to indent the #ifdef to be consistent with the other usages in these 
functions.

That's mostly it. I notice that only a small number of tests have been 
updated. Are there more test updates to come? I'm pretty sure we have a 
lot more tests that may require update (searching for SunOS might give 
some hints).

-Alan.







-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20131121/62f03fbc/attachment.html