<div dir="ltr">We've synced our staging repository yesterday with the latest jdk8-b117 and I noticed that change "8025985: com.sun.management.OSMBeanFactory should not be public" moved the file src/solaris/native/com/sun/<div id=":m1">
management/UnixOperatingSystem_md.c to src/solaris/native/sun/management/OperatingSystemImpl.c. Fortunately, my changes to UnixOperatingSystem_md.c described in the webrev apply cleanly to the new file (I've tested this locally). <br>
<br></div><div id=":m1">I'll update the webrev accordingly once I've collected some more feedback.<br><br></div><div id=":m1">Thank you and best regards,<br></div><div id=":m1">Volker<br></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"><<a href="mailto:volker.simonis@gmail.com" target="_blank">volker.simonis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr"><div><div>Hi,<br><br>this is the second review round for "<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>". 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.<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'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 "jdk/src/aix" 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
"<code>LD_LIBRARY_PATH</code>" directly to cope with different library path
names.</li><li>
Add <code>jre/lib/<arch>/jli</code> to the standard library search
path on AIX because the AIX linker doesn'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 '/proc/self' - 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'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've added the initialization of
some previously uninitialized data structures.</li><li>
Also, on AIX we don'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'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 "socket extensions for multicast source filters" 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. '9') and sets
'result' to NULL for the directory stream end. Otherwise, 'errno'
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
"C" locale as <code>isEnglish()</code>, ignore VM-warnings in the
output, make sure the "grandchild" processes created by the test
don'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'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'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'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've removed all the references to
OS/400 now.
</p><br></div><div><span></span><span></span></div></div></div>
</blockquote></div><br></div></div></div>