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

[Volker Simonis]

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.Bateman at 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
>
>
>