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

[Alan Bateman]

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 
> (http://pubs.opengroup.org/onlinepubs/000095399/functions/malloc.html) 
> "...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
>     initialization?
>
>
> 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.
Thanks.

>     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.
Thanks.


> :
>
>
> 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.
Okay.

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.

-Alan