<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
On 20/11/2013 18:26, Volker Simonis wrote:
<blockquote
cite="mid:CA+3eh11+1xUpTSmQti0aCWbBtKpW8AJ4n6VV3fk0J4uGc6xBzA@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>
<div>Hi,<br>
<br>
this is the second review round for "<a
moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8024854">8024854:
Basic changes and files to build the class library on AIX</a>".
The previous reviews can be found at the end of this mail in
the references section. <br>
<br>
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.
<div><span class=""></span><span class=""></span></div>
</div>
</div>
</div>
</blockquote>
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.<br>
<br>
I've skimmed through all the non-client files in the webrev and just
have a few comments:<br>
<br>
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.<br>
<br>
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.<br>
<br>
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?<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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).<br>
<br>
-Alan.<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
</body>
</html>