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



Reply via email to