[OpenJDK 2D-Dev] RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

Volker Simonis volker.simonis at gmail.com
Tue Nov 26 01:57:17 PST 2013


Hi Phil,

thanks a lot for the review. Please find my comments inline:


On Tue, Nov 26, 2013 at 12:24 AM, Phil Race <philip.race at oracle.com> wrote:

> Hi,
> I see you've already received a ton of good feedback on this v2.
>
> I have just a few  things to add.
> I don't know what symlinks might exist on AIX but it seems odd to
> me that you have :-
>
> 138 static char *fullAixFontPath[] = {
>  139     "/usr/lpp/X11/lib/X11/fonts/Type1",
> ..
>
> but the paths in the new file aix.fontconfig.properties look like this :-
>
> /usr/lib/X11/fonts/Type1/cour.pfa
>
>
You're absolutely right. I've updated 'aix.fontconfig.properties' to
contain the same absolute path like 'fontpath.c'. I've also added a comment
which describes to which AIX-package the fonts belong to and to which
locations they are sym-linked to.

Also for the build to pick up a file called
> *src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
>
> *it seems like you should have added a section to handle this path to
> jdk/makefiles/gendata/GenDataFontConfig.gmk
>
> That seems to be where the new build handles such files.
>
> Are you seeing the .bfc and .src files created ?
>
>
You're right. But that was already added by the general AIX-build change
"8024900: PPC64: Enable new build on AIX (jdk part)" (
http://hg.openjdk.java.net/ppc-aix-port/stage/jdk/diff/3ac08cd5e2e8/makefiles/GendataFontConfig.gmk
).

And yes, the .bfc and .src files are created and copied to the right places.


> At runtime it'll get picked up so so long as System.getProperty("os.name")
> returns "aix" (all lower-case) so I think that's OK. Its the build time
> part
> I'd expect to see but don't.
>
>
I did split the AIX change into several changes and the build changes have
been reviewed separately:

8024265: PPC64: Enable new build on AIX (
https://bugs.openjdk.java.net/browse/JDK-8024265)
8024900: PPC64: Enable new build on AIX (jdk part) (
https://bugs.openjdk.java.net/browse/JDK-8024900)

This change only contains the additional make changes which became
necessary after I started to move AIX-specific files into their own
jdk/src/aix/ directory. Everything else is already in place.

I'll prepare and test a finaly webrev with all the changes from this review
round today.

Thanks a lot,
Volker



> -phil.
>
>
> On 11/20/2013 10:26 AM, 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/ <
>> http://cr.openjdk.java.net/%7Esimonis/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.properties|for 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/DefaultAsynchronousChannelProv
>> ider.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.c| because 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.
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/security-dev/attachments/20131126/3276cd8c/attachment-0001.html 


More information about the security-dev mailing list