Hi Alan, so I'll rename 'initLocalAddrTable()' into 'platformInit()' and move the call to 'aix_close_init' to a AIX-specific version of 'platformInit()' in net_util_md.c as discussed.
I'll post a final webrev once I got the comments from the AWT/2D guys. As far as I understood, you've now reviewed the 'core-lib'/'net' parts right? That would mean that I'll still need a review from the AWT/2D and the Security group - any volunteers:). Once again thanks a lot for your help, Volker On Fri, Nov 22, 2013 at 2:24 PM, Alan Bateman <alan.bate...@oracle.com> wrote: > 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 > > >