Hi Sean, thanks a lot for you review.
Please let me know once you start working on 6997010 so I can update the corresponding AIX file accordingly. Regards, Volker On Monday, November 25, 2013, Sean Mullan wrote: > Hi Volker, > > The security changes look fine. I'm not crazy that we now have to maintain > one additional java.security file which is the exact same as > java.security-linux, but this is really an existing issue with duplicated > content across the java.security files which I will try to fix early in JDK > 9: https://bugs.openjdk.java.net/browse/JDK-6997010 > > Thanks, > Sean > > On 11/20/2013 01:26 PM, 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. >> >> The webrev is against our >> http://hg.openjdk.java.net/ppc-aix-port/stagerepository which has been >> recently synchronised with the jdk8 master: >> >> http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/ >> >> 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:) >> >> Thank you and best regards, >> Volker >> >> *References:* >> >> The following reviews have been posted so far (thanks Iris for collecting >> them :) >> >> - Alan Bateman (lib): Initial comments (16 Sep [2]) >> - Chris Hegarty (lib/net): Initial comments (20 Sep [3]) >> - Michael McMahon (net): Initial comments (20 Sept [4]) >> - Steffan Larsen (svc): APPROVED (20 Sept [5]) >> - Phil Race (2d): Initial comments (18 Sept [6]); Additional comments (15 >> Oct [7]) >> - Sean Mullan (sec): Initial comments (26 Sept [8]) >> >> [2]: >> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/ >> 2013-September/001045.html >> [3]: >> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/ >> 2013-September/001078.html >> [4]: >> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/ >> 2013-September/001079.html >> [5]: >> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/ >> 2013-September/001077.html >> [6]: >> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/ >> 2013-September/001069.html >> [7]: http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/ >> 003826.html >> [8]: >> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/ >> 2013-September/001081.html >> >> >> *Detailed change description:* >> >> The new "jdk/src/aix" subdirectory contains the following new and >> AIX-specific files for now: >> >> 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 >> >> src/aix/porting/porting_aix.c >> src/aix/porting/porting_aix.h >> >> - Added these two files for AIX relevant utility code. >> - Currently these files only contain an implementation of dladdr which >> is not available on AIX. >> - In the first review round the existing dladdr users in the code >> either >> called the version from the HotSpot on AIX ( >> src/solaris/native/sun/awt/awt_LoadLibrary.c) or had a private copy >> of >> the whole implementation (src/solaris/demo/jvmti/hprof/hprof_md.c). >> This >> is now not necessary any more. >> >> The new file layout required some small changes to the makefiles to make >> them aware of the new directory locations: >> >> makefiles/CompileDemos.gmk >> >> - Add an extra argument to SetupJVMTIDemo which can be used to pass >> additional source locations. >> - Currently this is only used on AIX for the AIX porting utilities >> which >> are required by hprof. >> >> makefiles/lib/Awt2dLibraries.gmk >> makefiles/lib/ServiceabilityLibraries.gmk >> >> - On AIX add the sources of the AIX porting utilities to the build. >> They >> are required by src/solaris/native/sun/awt/awt_LoadLibrary.c and >> src/solaris/demo/jvmti/hprof/hprof_md.c because dladdr is not >> available >> on AIX. >> >> makefiles/lib/NioLibraries.gmk >> >> - Make the AIX-build aware of the new NIO source locations under >> src/aix/native/sun/nio/. >> >> makefiles/lib/NetworkingLibraries.gmk >> >> - Make the AIX-build aware of the new aix_close.c source location >> under >> src/aix/native/java/net/. >> >> src/share/bin/jli_util.h >> >> - Define JLI_Lseek on AIX. >> >> src/share/lib/security/java.security-aix >> >> - Provide default java.security-aix for AIX based on the latest Linux >> version as suggested by Sean Mullan. >> >> src/share/native/common/check_code.c >> >> - On AIX malloc(0) and calloc(0, ...) return a NULL pointer, which is >> legal, but the code in check_code.c 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. >> >> src/share/native/sun/awt/medialib/mlib_sys.c >> >> - malloc always returns 8-byte aligned pointers on AIX as well. >> >> src/share/native/sun/awt/medialib/mlib_types.h >> >> - Add AIX to the list of known platforms. >> >> src/share/native/sun/font/layout/KernTable.cpp >> >> - Rename the macro DEBUG to DEBUG_KERN_TABLE because DEBUG is too >> common >> and may be defined in other headers as well as on the command line and >> xlc bails out on macro redefinitions with a different value. >> >> src/share/native/sun/security/ec/impl/ecc_impl.h >> >> - Define required types and macros on AIX. >> >> src/solaris/back/exec_md.c >> >> - AIX behaves like Linux in this case so check for it in the Linux >> branch. >> >> src/solaris/bin/java_md_solinux.c, >> src/solaris/bin/java_md_solinux.h >> >> - On AIX LD_LIBRARY_PATH is called LIBPATH >> - Always use LD_LIBRARY_PATH macro instead of using the string " >> LD_LIBRARY_PATH" directly to cope with different library path names. >> - Add jre/lib/<arch>/jli to the standard library search path on AIX >> because the AIX linker doesn't support the -rpath option. >> - Replace #ifdef __linux__ by #ifndef __solaris__ because in this >> case, >> AIX behaves like Linux. >> - Removed the definition of JVM_DLL, JAVA_DLL and LD_LIBRARY_PATH from >> java_md_solinux.h because the macros are redefined in the >> corresponding >> .c files anyway. >> >> src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties >> >> - Provide basic fontconfig.propertiesfor AIX. >> >> src/solaris/classes/java/lang/UNIXProcess.java.aix, >> src/aix/classes/sun/tools/attach/AixAttachProvider.java, >> src/aix/classes/sun/tools/attach/AixVirtualMachine.java, >> src/aix/native/sun/tools/attach/AixVirtualMachine.c >> >> - Provide AIX specific Java versions, mostly based on the >> corresponding >> Linux versions. >> >> src/solaris/classes/sun/nio/ch/DefaultAsynchronousChannelProvider.java >> src/solaris/classes/sun/nio/fs/DefaultFileSystemProvider.java >> >> - Detect AIX operating system and return the corresponding channel and >> file system providers. >> >> src/solaris/classes/sun/nio/ch/Port.java >> >> - Add a callback function unregisterImpl(int fd) for implementations >> that need special handling when fd is removed and call it from >> unregister(int >> fd). By default the implementation of unregisterImpl(int fd) is empty >> except for the derived AixPollPort class on AIX. >> >> src/solaris/demo/jvmti/hprof/hprof_md.c >> >> - Add AIX support. AIX mostly behaves like Linux, with the one >> exception >> that it has no dladdr functionality. >> - Use the dladdr implementation from porting_aix.{c,h} on AIX. >> >> src/solaris/native/com/sun/management/UnixOperatingSystem_md.c >> >> - Adapt for AIX (i.e. use libperfstat on AIX to query OS memory). >> >> src/solaris/native/common/jdk_util_md.h >> >> - Add AIX definitions of the ISNANF and ISNAND macros. >> >> src/solaris/native/java/io/io_util_md.c >> >> - AIX behaves like Linux in this case so check for it in the Linux >> branch. >> >> src/solaris/native/java/lang/UNIXProcess_md.c >> >> - AIX behaves mostly like Solraris in this case so adjust the ifdefs >> accordingly. >> >> src/solaris/native/java/lang/childproc.c >> >> - AIX does not understand '/proc/self' - it requires the real process >> ID >> to query the proc file system for the current process. >> >> src/solaris/native/java/net/NetworkInterface.c >> >> - Add AIX support into the Linux branch because AIX mostly behaves >> like >> AIX for IPv4. >> - AIX needs a special function to enumerate IPv6 interfaces and to >> query >> the MAC address. >> - Moved the declaration of siocgifconfRequest to the beginning a the >> function (as recommend by Michael McMahon) to remain C89 compatible. >> >> src/solaris/native/java/net/PlainSocketImpl.c >> >> - On AIX (like on Solaris) setsockopt will set errno to EINVAL if the >> socket is closed. The default error message is then confusing. >> >> src/aix/native/java/net/aix_close.c, >> src/share/native/java/net/net_util.c >> >> - As recommended by Michael McMahon and Alan Bateman I've move an >> adapted version of linux_close.c to >> src/aix/native/java/net/aix_close.cbecause we also need the file and >> socket wrappers on AIX. >> - Compared to the Linux version, we've added the initialization of >> some >> previously uninitialized data structures. >> - Also, on AIX we don't have __attribute((constructor)) so we need to >> initialize manually (from JNI_OnLoad() in >> src/share/native/java/net/net_util.c. >> >> src/solaris/native/java/net/net_util_md.h >> >> - AIX needs the same workaround for I/O cancellation like Linux and >> MacOSX. >> >> src/solaris/native/java/net/net_util_md.c >> >> - SO_REUSEADDR is called SO_REUSEPORT on AIX. >> - On AIX we have to ignore failures due to ENOBUFS when setting the >> SO_SNDBUF/SO_RCVBUF socket options. >> >> src/solaris/native/java/util/TimeZone_md.c >> >> - Currently on AIX the only way to get the platform time zone is to >> read >> it from the TZ environment variable. >> >> src/solaris/native/sun/awt/awt_LoadLibrary.c >> >> - Use the dladdr from porting_aix.{c,h} on AIX. >> >> src/solaris/native/sun/awt/fontpath.c >> >> - Changed some macros from if !defined(__linux__) to if >> defined(__solaris__) because that's their real meaning. >> - Add AIX specific fontpath settings and library search paths for >> libfontconfig.so. >> >> src/solaris/native/sun/java2d/x11/X11SurfaceData.c >> >> - Rename the MIN and MAX macros to XSD_MIN and XSD_MAX to avoid name >> clashes (MIN and MAX are much too common and thexy are already defined >> in some AIX system headers). >> >> src/solaris/native/sun/java2d/x11/XRBackendNative.c >> >> - Handle AIX like Solaris. >> >> src/solaris/native/sun/nio/ch/Net.c >> >> - Add AIX-specific includes and constant definitions. >> - 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 >> >> src/solaris/native/sun/nio/fs/UnixNativeDispatcher.c >> >> - On AIX strerror() is not thread-safe so we have to use >> strerror_r()instead. >> - On AIX readdir_r() returns EBADF (i.e. '9') and sets 'result' to >> NULL >> for the directory stream end. Otherwise, 'errno' will contain the >> error >> code. >> - Handle some AIX corner cases for the results of the statvfs64() >> call. >> - On AIX getgrgid_r() returns ESRCH if group does not exist. >> >> src/solaris/native/sun/security/pkcs11/j2secmod_md.c >> >> - Use RTLD_LAZY instead of RTLD_NOLOAD on AIX. >> >> test/java/lang/ProcessBuilder/Basic.java >> test/java/lang/ProcessBuilder/DestroyTest.java >> >> - Port this test to AIX and make it more robust (i.e. recognize the >> "C" >> locale as isEnglish(), 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). >> >> *Q (Michael McMahon):* Seems to be two macros _AIX and AIX. Is this >> intended? >> >> Well, _AIX is defined in some system headers while AIX is defined by the >> build system. This is already used inconsistently (i.e. __linux__ vs. >> LINUX) >> 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 AIX to >> _AIX. >> >> >> *Q (Alan Bateman):* There are a few changes for OS/400 in the patch, are >> they supposed to be there? >> >> 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. >> >> >
