RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX
Alan.Bateman at oracle.com
Fri Nov 22 13:24:56 UTC 2013
On 21/11/2013 15:54, Volker Simonis wrote:
> But actually I've just realized that it is not need at all, because
> 'aix_close.c' isn't in the PATH for any other OS than AIX (that could
> be probably called a feature of the new file layout:) So I'll simply
> change it to:
> 48 ifeq ($(OPENJDK_TARGET_OS), aix)
> 49 LIBNET_SRC_DIRS += $(JDK_TOPDIR)/src/aix/native/java/net/
> 50 endif
This make sense.
> Yes, exactly. I didn't wanted to change too much code. But as the
> C-Standard states
> "...If size is 0, either a null pointer or a unique pointer that can
> be successfully passed to free() shall be returned..." it is perfectly
> legal that malloc/calloc return a NULL pointer if called with a zero
> argument. This case is currently not handled (i.e. it's handled as an
> 'out of memory' error) in check_code.c and I agree that this should be
> fixed via a separate bug.
Yes, let's use a separate bug for this.
> 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
> What about renaming 'initLocalAddrTable()' into something like
> 'platformInit()' and moving the call to 'aix_close_init' to a
> AIX-specific version of 'platformInit()' in net_util_md.c?
I don't have a strong opinion on the name of the function, platformInit
is fine for now.
> You're right - we could rename it to something like 'java_md_unix.c'.
> But no matter how fancy the name would be, the file would still be in
> the 'src/solaris/bin' subdirectory:( So I think we'd better leave this
> for a later change when we completely factor out the Linux/Mac code
> from the 'solaris/' directory.
I think JDK 9 is a good time to finally tackle this issue (as you
probably know, this is something that I've brought up on porters-dev or
build-dev a few times).
> 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.
> Done. Also changed the comment to JavaDoc style to be more consistent
> with the other comments in that file.
> 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.
> You're right. This change was done before you fixed "7043788: (fs)
> PosixFileAttributes.owner() or group() throws NPE if owner/group not
> in passwd/group database"
> (http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/f91c799f7bfb). After
> you're fix it was "automatically" adapted. I've removed the special
> AIX handling as suggested because I think as well that another error
> message in the exception won't have any impact.
> I'm currently working on it and created "8028537: PPC64: Updated
> jdk/test scripts to understand the AIX os and environment" for it
> because I didn't wanted to blow up this change unnecessarily.
So overall I think this patch is in good shape (I have not reviewed the
AWT/2D/client changes in any detail) and I don't see any blocking issues
to this going in.
More information about the core-libs-dev