<div dir="ltr"><div><div><div><div><div><div>Hi,<br><br></div>thanks to everybody for the prompt and helpful reviews. Here comes the final webrev which incorporates all the corrections and suggestions from the second review round:<br>
<br><a href="http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/">http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/</a><br><br></div>I&#39;ve successfully build (and run some smoke tests) with these changes on Linux (x86_32, x86_64, ppc64), Solaris/sparcv9, Windows/x86_64, MacOSX and AIX (5.3, 7.1).<br>
<br></div><div>So if nobody vetoes these changes are ready for push into our staging repository.<br><br></div><div>@Vladimir: can I push them or do you want to run them trough JPRT? <br></div><div><br>Thank you and best regards,<br>
Volker<br><br>PS: compared to the last webrev (<a href="http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/">http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/</a>), I&#39;ve slightly changed the following files:<br>
<br><span style="font-family:courier new,monospace">makefiles/lib/Awt2dLibraries.gmk<br>makefiles/lib/NetworkingLibraries.gmk<br>makefiles/Setup.gmk<br>src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties<br>src/aix/classes/sun/nio/ch/AixPollPort.java<br>
src/aix/classes/sun/nio/fs/AixFileSystem.java<br>src/aix/native/java/net/aix_close.c<br>src/aix/porting/porting_aix.c<br>src/share/native/java/net/net_util.c<br>src/share/native/java/net/net_util.h<br>src/share/native/sun/awt/medialib/mlib_sys.c<br>
src/solaris/bin/java_md_solinux.c<br>src/solaris/classes/sun/nio/ch/Port.java<br>src/solaris/native/java/net/net_util_md.c<br>src/solaris/native/sun/java2d/x11/XRBackendNative.c<br>src/solaris/native/sun/management/OperatingSystemImpl.c<br>
src/solaris/native/sun/nio/fs/UnixNativeDispatcher.c<br>src/windows/native/java/net/net_util_md.c</span><br><br></div></div>Most of the changes are cosmetic, except the ones in:<br><br><span style="font-family:courier new,monospace">src/share/native/java/net/net_util.c<br>
src/share/native/java/net/net_util.h<br>src/solaris/native/java/net/net_util_md.c<br>src/windows/native/java/net/net_util_md.c</span><br><br></div>where I renamed &#39;initLocalAddrTable()&#39; to &#39;platformInit()&#39;. For AIX, this method will now call &#39;aix_close_init()&#39; as suggested by Alan.<br>
<br></div>The changes to src/solaris/native/com/sun/
management/UnixOperatingSystem_md.c are now in src/solaris/native/sun/management/OperatingSystemImpl.c because that file was moved by an upstream change.<br><div><div><br></div></div></div><div class="gmail_extra"><br><br>
<div class="gmail_quote">On Wed, Nov 20, 2013 at 7:26 PM, Volker Simonis <span dir="ltr">&lt;<a href="mailto:volker.simonis@gmail.com" target="_blank">volker.simonis@gmail.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div><div>Hi,<br><br>this is the second review round for &quot;<a href="https://bugs.openjdk.java.net/browse/JDK-8024854" target="_blank">8024854: Basic changes and files to build the class library on AIX</a>&quot;. The previous reviews can be found at the end of this mail in the references section. <br>

<br>I&#39;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 &quot;aix/&quot; subdirectory which I&#39;ve now created under &quot;jdk/src&quot; and which contains AIX-only code.<br>

<br></div><div>The webrev is against our <a href="http://hg.openjdk.java.net/ppc-aix-port/stage" target="_blank">http://hg.openjdk.java.net/ppc-aix-port/stage</a> repository which has been recently synchronised with the jdk8 master:<br>
<br>
<a href="http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/" target="_blank">http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/</a><br></div><div><br></div><div>Below (and in the webrev) you can find some comments which explain the changes to each file. In order to be able to push this big change, I need the approval of reviewers from the lib, net, svc, 2d, awt and sec groups. So please don&#39;t hesitate to jump at it - there are enough changes for everybody:)<br>

<br></div><div>Thank you and best regards,<br></div><div>Volker<br></div><div><br></div><b>References:</b><br></div><div><br></div><div>The following reviews have been posted so far (thanks Iris for collecting them :)<br>

</div><div><div><br>- Alan Bateman (lib): Initial comments (<span><span>16 Sep</span></span> [2])<br>
- Chris Hegarty (lib/net): Initial comments (<span><span>20 Sep</span></span> [3])<br>
- Michael McMahon (net): Initial comments (<span><span>20 Sept</span></span> [4])<br>
- Steffan Larsen (svc): APPROVED (<span><span>20 Sept</span></span> [5])<br>
- Phil Race (2d): Initial comments  (<span><span>18 Sept</span></span> [6]); Additional comments (15 Oct [7])<br>
- Sean Mullan (sec): Initial comments (<span><span>26 Sept</span></span> [8])<br>
<br>[2]: <a href="http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html" target="_blank">http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html</a><br>
[3]: <a href="http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html" target="_blank">http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html</a><br>
[4]: <a href="http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html" target="_blank">http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html</a><br>
[5]: <a href="http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html" target="_blank">http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html</a><br>
[6]: <a href="http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html" target="_blank">http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html</a><br>
[7]: <a href="http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html" target="_blank">http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html</a><br>
[8]: <a href="http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html" target="_blank">http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html</a><br><br></div><div><br>

</div><div><b>Detailed change description:</b><br><br></div><div>The new &quot;jdk/src/aix&quot; subdirectory contains the following new and AIX-specific files for now:<br><pre><font> src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
 src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
 src/aix/classes/sun/nio/ch/AixPollPort.java
 src/aix/classes/sun/nio/fs/AixFileStore.java
 src/aix/classes/sun/nio/fs/AixFileSystem.java
 src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
 src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
 src/aix/classes/sun/tools/attach/AixAttachProvider.java
 src/aix/classes/sun/tools/attach/AixVirtualMachine.java
 src/aix/native/java/net/aix_close.c
 src/aix/native/sun/nio/ch/AixPollPort.c
 src/aix/native/sun/nio/fs/AixNativeDispatcher.c
 src/aix/native/sun/tools/attach/AixVirtualMachine.c
 src/aix/porting/porting_aix.c
 src/aix/porting/porting_aix.h</font>
</pre>

<h4>src/aix/porting/porting_aix.c<br>
    src/aix/porting/porting_aix.h</h4>

<ul style="padding-left:1em"><li>
    Added these two files for AIX relevant utility code.</li><li>
    Currently these files only contain an implementation
    of <code>dladdr</code> which is not available on AIX.</li><li>
    In the first review round the existing <code>dladdr</code> users
    in the code either called the version from the HotSpot on AIX
    (<code>src/solaris/native/sun/awt/awt_LoadLibrary.c</code>) or had
    a private copy of the whole implementation
    (<code>src/solaris/demo/jvmti/hprof/hprof_md.c</code>). This is
    now not necessary any more.
</li></ul>


<p>
The new file layout required some small changes to the makefiles to
make them aware of the new directory locations:
</p><p>

</p><h4>makefiles/CompileDemos.gmk</h4>

<ul style="padding-left:1em"><li>
    Add an extra argument to <code>SetupJVMTIDemo</code> which can be
    used to pass additional source locations.</li><li>
    Currently this is only used on AIX for the AIX porting utilities
    which are required by hprof.</li></ul>

<h4>makefiles/lib/Awt2dLibraries.gmk<br>
    makefiles/lib/ServiceabilityLibraries.gmk</h4>

<ul style="padding-left:1em"><li>
    On AIX add the sources of the AIX porting utilities to the
    build. They are required
    by <code>src/solaris/native/sun/awt/awt_LoadLibrary.c</code>
    and <code>src/solaris/demo/jvmti/hprof/hprof_md.c</code>
    because <code>dladdr</code> is not available on AIX.</li></ul>

<h4>makefiles/lib/NioLibraries.gmk</h4>

<ul style="padding-left:1em"><li>
    Make the AIX-build aware of the new NIO source locations
    under <code>src/aix/native/sun/nio/</code>.</li></ul>

<h4>makefiles/lib/NetworkingLibraries.gmk</h4>

<ul style="padding-left:1em"><li>
    Make the AIX-build aware of the new <code>aix_close.c</code> source location
    under <code>src/aix/native/java/net/</code>.</li></ul>

<h4>src/share/bin/jli_util.h</h4>

<ul style="padding-left:1em"><li>
    Define <code>JLI_Lseek</code> on AIX.</li></ul>

<h4>src/share/lib/security/java.security-aix</h4>

<ul style="padding-left:1em"><li>
    Provide default <code>java.security-aix</code> for AIX based on
    the latest Linux version as suggested by Sean Mullan.</li></ul>

<h4>src/share/native/common/check_code.c</h4>

<ul style="padding-left:1em"><li>
    On AIX malloc(0) and calloc(0, ...) return a NULL pointer, which
    is legal, but the code in <code>check_code.c</code> does not
    handles this properly. So we wrap the two methods on AIX and
    return a non-NULL pointer even if we allocate 0 bytes.</li></ul>

<h4>src/share/native/sun/awt/medialib/mlib_sys.c</h4>

<ul style="padding-left:1em"><li>
    <code>malloc</code> always returns 8-byte aligned pointers on AIX as well.</li></ul>

<h4>src/share/native/sun/awt/medialib/mlib_types.h</h4>

<ul style="padding-left:1em"><li>
    Add AIX to the list of known platforms.</li></ul>

<h4>src/share/native/sun/font/layout/KernTable.cpp</h4>

<ul style="padding-left:1em"><li>
    Rename the macro <code>DEBUG</code> to <code>DEBUG_KERN_TABLE</code>
    because <code>DEBUG</code> is too common and may be defined in other
    headers as well as on the command line and <code>xlc</code> bails out on
    macro redefinitions with a different value.</li></ul>

<h4>src/share/native/sun/security/ec/impl/ecc_impl.h</h4>

<ul style="padding-left:1em"><li>
    Define required types and macros on AIX.</li></ul>

<h4>src/solaris/back/exec_md.c</h4>

<ul style="padding-left:1em"><li>
    AIX behaves like Linux in this case so check for it in the Linux branch.</li></ul>

<h4>src/solaris/bin/java_md_solinux.c,<br>
    src/solaris/bin/java_md_solinux.h</h4>

<ul style="padding-left:1em"><li>
    On AIX <code>LD_LIBRARY_PATH</code> is called <code>LIBPATH</code></li><li>
    Always use <code>LD_LIBRARY_PATH</code> macro instead of using the string
    &quot;<code>LD_LIBRARY_PATH</code>&quot; directly to cope with different library path
    names.</li><li>
    Add <code>jre/lib/&lt;arch&gt;/jli</code> to the standard library search
    path on AIX because the AIX linker doesn&#39;t support the <code>-rpath</code>
    option.</li><li>
    Replace <code>#ifdef __linux__</code> by <code>#ifndef __solaris__</code>
    because in this case, AIX behaves like Linux.</li><li>
    Removed the definition
    of <code>JVM_DLL</code>, <code>JAVA_DLL</code>
    and <code>LD_LIBRARY_PATH</code>
    from <code>java_md_solinux.h</code> because the macros are
    redefined in the corresponding <code>.c</code> files anyway.
</li></ul>

<h4>src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties</h4>

<ul style="padding-left:1em"><li>
    Provide basic <code>fontconfig.properties</code>for AIX.</li></ul>

<h4>src/solaris/classes/java/lang/UNIXProcess.java.aix,<br>
    src/aix/classes/sun/tools/attach/AixAttachProvider.java,<br>
    src/aix/classes/sun/tools/attach/AixVirtualMachine.java,<br>
    src/aix/native/sun/tools/attach/AixVirtualMachine.c</h4>

<ul style="padding-left:1em"><li>
    Provide AIX specific Java versions, mostly based on the corresponding
    Linux versions.</li></ul>

<h4>src/solaris/classes/sun/nio/ch/DefaultAsynchronousChannelProvider.java<br>
    src/solaris/classes/sun/nio/fs/DefaultFileSystemProvider.java</h4>

<ul style="padding-left:1em"><li>
    Detect AIX operating system and return the corresponding channel
    and file system providers.</li></ul>

<h4>src/solaris/classes/sun/nio/ch/Port.java</h4>

<ul style="padding-left:1em"><li>
    Add a callback function <code>unregisterImpl(int fd)</code> for
    implementations that need special handling when <code>fd</code> is
    removed and call it from <code>unregister(int fd)</code>. By
    default the implementation of <code>unregisterImpl(int fd)</code>
    is empty except for the derived <code>AixPollPort</code> class on
    AIX.</li></ul>

<h4>src/solaris/demo/jvmti/hprof/hprof_md.c</h4>

<ul style="padding-left:1em"><li>
    Add AIX support. AIX mostly behaves like Linux, with the one exception
    that it has no <code>dladdr</code> functionality.</li><li>
    Use the <code>dladdr</code> implementation
    from <code>porting_aix.{c,h}</code> on AIX.</li></ul>

<h4>src/solaris/native/com/sun/management/UnixOperatingSystem_md.c</h4>

<ul style="padding-left:1em"><li>
    Adapt for AIX (i.e. use <code>libperfstat</code> on AIX to query OS memory).</li></ul>

<h4>src/solaris/native/common/jdk_util_md.h</h4>

<ul style="padding-left:1em"><li>
    Add AIX definitions of the <code>ISNANF</code> and <code>ISNAND</code> macros.</li></ul>

<h4>src/solaris/native/java/io/io_util_md.c</h4>

<ul style="padding-left:1em"><li>
    AIX behaves like Linux in this case so check for it in the Linux branch.</li></ul>

<h4>src/solaris/native/java/lang/UNIXProcess_md.c</h4>

<ul style="padding-left:1em"><li>
    AIX behaves mostly like Solraris in this case so adjust the ifdefs
    accordingly.</li></ul>

<h4>src/solaris/native/java/lang/childproc.c</h4>

<ul style="padding-left:1em"><li>
    AIX does not understand &#39;/proc/self&#39; - it requires the real
    process ID to query the proc file system for the current process.</li></ul>

<h4>src/solaris/native/java/net/NetworkInterface.c</h4>

<ul style="padding-left:1em"><li>
    Add AIX support into the Linux branch because AIX mostly behaves like AIX
    for IPv4.</li><li>
    AIX needs a special function to enumerate IPv6 interfaces and to query the
    MAC address.</li><li>
    Moved the declaration of <code>siocgifconfRequest</code> to the
    beginning a the function (as recommend by Michael McMahon) to remain
    C89 compatible.</li></ul>

<h4>src/solaris/native/java/net/PlainSocketImpl.c</h4>

<ul style="padding-left:1em"><li>
    On AIX (like on Solaris) <code>setsockopt</code> will set errno
    to <code>EINVAL</code> if the socket is closed. The default error message
    is then confusing.</li></ul>

<h4>src/aix/native/java/net/aix_close.c,<br>
    src/share/native/java/net/net_util.c</h4>

<ul style="padding-left:1em"><li>
    As recommended by Michael McMahon and Alan Bateman I&#39;ve move an
    adapted version of <code>linux_close.c</code>
    to <code>src/aix/native/java/net/aix_close.c</code> because we
    also need the file and socket wrappers on AIX.</li><li>
    Compared to the Linux version, we&#39;ve added the initialization of
    some previously uninitialized data structures.</li><li>
    Also, on AIX we don&#39;t have <code>__attribute((constructor))</code>
    so we need to initialize manually (from <code>JNI_OnLoad()</code>
    in <code>src/share/native/java/net/net_util.c</code>.
</li></ul>

<h4>src/solaris/native/java/net/net_util_md.h</h4>

<ul style="padding-left:1em"><li>
    AIX needs the same workaround for I/O cancellation like Linux and MacOSX.</li></ul>

<h4>src/solaris/native/java/net/net_util_md.c</h4>

<ul style="padding-left:1em"><li>
    <code>SO_REUSEADDR</code> is called <code>SO_REUSEPORT</code> on AIX.</li><li>
    On AIX we have to ignore failures due to <code>ENOBUFS</code>
    when setting the <code>SO_SNDBUF</code>/<code>SO_RCVBUF</code>
    socket options.</li></ul>

<h4>src/solaris/native/java/util/TimeZone_md.c</h4>

<ul style="padding-left:1em"><li>
    Currently on AIX the only way to get the platform time zone is to read it
    from the <code>TZ</code> environment variable.</li></ul>

<h4>src/solaris/native/sun/awt/awt_LoadLibrary.c</h4>

<ul style="padding-left:1em"><li>
    Use the <code>dladdr</code> from <code>porting_aix.{c,h}</code> on
    AIX.</li></ul>

<h4>src/solaris/native/sun/awt/fontpath.c</h4>

<ul style="padding-left:1em"><li>
    Changed some macros from <code>if !defined(__linux__)</code>
    to <code>if defined(__solaris__)</code> because that&#39;s their real
    meaning.</li><li>
    Add AIX specific fontpath settings and library search paths
    for <code>libfontconfig.so</code>.</li></ul>

<h4>src/solaris/native/sun/java2d/x11/X11SurfaceData.c</h4>

<ul style="padding-left:1em"><li>
    Rename the <code>MIN</code> and <code>MAX</code> macros
    to <code>XSD_MIN</code> and <code>XSD_MAX</code> to avoid name
    clashes (<code>MIN</code> and <code>MAX</code> are much too common
    and thexy are already defined in some AIX system headers).</li></ul>

<h4>src/solaris/native/sun/java2d/x11/XRBackendNative.c</h4>

<ul style="padding-left:1em"><li>
    Handle AIX like Solaris.</li></ul>

<h4>src/solaris/native/sun/nio/ch/Net.c</h4>

<ul style="padding-left:1em"><li>
    Add AIX-specific includes and constant definitions.</li><li>
    On AIX &quot;socket extensions for multicast source filters&quot; support
    depends on the OS version. Check for this and throw appropriate
    exceptions if it is requested but not supported. This is needed to
    pass
    JCK-api/java_nio/channels/DatagramChannel/DatagramChannel.html#Multicast</li></ul>





<h4>src/solaris/native/sun/nio/fs/UnixNativeDispatcher.c</h4>

<ul style="padding-left:1em"><li>
    On AIX <code>strerror()</code> is not thread-safe so we have to
    use <code>strerror_r()</code> instead.</li><li>
    On AIX <code>readdir_r()</code> returns EBADF (i.e. &#39;9&#39;) and sets
    &#39;result&#39; to NULL for the directory stream end. Otherwise, &#39;errno&#39;
    will contain the error code.</li><li>
    Handle some AIX corner cases for the results of
    the <code>statvfs64()</code> call.</li><li>
    On AIX <code>getgrgid_r()</code> returns ESRCH if group does not exist.</li></ul>

<h4>src/solaris/native/sun/security/pkcs11/j2secmod_md.c</h4>

<ul style="padding-left:1em"><li>
    Use <code>RTLD_LAZY</code> instead of <code>RTLD_NOLOAD</code> on AIX.</li></ul>

<h4>test/java/lang/ProcessBuilder/Basic.java<br>
    test/java/lang/ProcessBuilder/DestroyTest.java</h4>

<ul style="padding-left:1em"><li>
    Port this test to AIX and make it more robust (i.e. recognize the
    &quot;C&quot; locale as <code>isEnglish()</code>, ignore VM-warnings in the
    output, make sure the &quot;grandchild&quot; processes created by the test
    don&#39;t run too long (60s vs. 6666s) because in the case of test
    problems these processes will pollute the process space, make sure
    the test fails with an error and doesn&#39;t hang indefinitley
    in the case of a problem).</li></ul>


<p>
<b>Q (Michael McMahon):</b> Seems to be two macros _AIX and AIX. Is this intended?
</p>
<p>
Well, <code>_AIX</code> is defined in some system headers
while <code>AIX</code> is defined by the build system. This is
already used inconsistently (i.e. <code>__linux__</code>
vs. <code>LINUX</code>) and in general I try to be consistent with the
style of the file where I the changes are. That said, I changes most
of the occurences of <code>AIX</code> to <code>_AIX</code>.
</p>


<p>
<b>Q (Alan Bateman):</b> There are a few changes for OS/400 in the patch, are they supposed to be there?
</p>
<p>
We currently don&#39;t support OS/400 (later renamed into i5/OS and currently called
IBM i) in our OpenJDK port but we do support it in our internel, SAP JVM
build. We stripped out all the extra OS/400 functionality from the port but in
some places where there is common functionality needd by both, AIX and OS/400
the OS/400 support may still be visible in the OpenJDK port. I don&#39;t think this
is a real problem and it helps us to keep our internel sources in sync with the
OpenJDK port. That said, I think I&#39;ve removed all the references to
OS/400 now.
</p><br></div><div><span></span><span></span></div></div></div>
</blockquote></div><br></div>