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.
