<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
On 21/11/2013 15:54, Volker Simonis wrote:
<blockquote
cite="mid:CA+3eh105jG=-g3czKvamJnnA_1vw5XAWM7puBKa-6caR3KAGVw@mail.gmail.com"
type="cite">
<div dir="ltr">:
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div>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:<br>
<pre><span class=""> 48 ifeq ($(OPENJDK_TARGET_OS), aix)</span>
<span class=""><span class=""> 49 LIBNET_SRC_DIRS += $(JDK_TOPDIR)/src/aix/native/java/net/</span>
50 endif</span></pre>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
This make sense.<br>
<br>
<br>
<blockquote
cite="mid:CA+3eh105jG=-g3czKvamJnnA_1vw5XAWM7puBKa-6caR3KAGVw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
Yes, exactly. I didn't wanted to change too much code.
But as the C-Standard states (<a
moz-do-not-send="true"
href="http://pubs.opengroup.org/onlinepubs/000095399/functions/malloc.html">http://pubs.opengroup.org/onlinepubs/000095399/functions/malloc.html</a>)
"...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.</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
Yes, let's use a separate bug for this.<br>
<br>
<br>
<br>
<blockquote
cite="mid:CA+3eh105jG=-g3czKvamJnnA_1vw5XAWM7puBKa-6caR3KAGVw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> 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?<br>
<br>
</div>
</blockquote>
<div><br>
</div>
<div>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?<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
I don't have a strong opinion on the name of the function,
platformInit is fine for now.<br>
<br>
<br>
<blockquote
cite="mid:CA+3eh105jG=-g3czKvamJnnA_1vw5XAWM7puBKa-6caR3KAGVw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div>
</div>
:<br>
<div bgcolor="#FFFFFF" text="#000000"><br>
</div>
<div><br>
</div>
<div>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.<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
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).<br>
<br>
<br>
<blockquote
cite="mid:CA+3eh105jG=-g3czKvamJnnA_1vw5XAWM7puBKa-6caR3KAGVw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> 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.<br>
<br>
</div>
</blockquote>
<div><br>
</div>
<div>Done. Also changed the comment to JavaDoc style to
be more consistent with the other comments in that
file.<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
Thanks.<br>
<br>
<blockquote
cite="mid:CA+3eh105jG=-g3czKvamJnnA_1vw5XAWM7puBKa-6caR3KAGVw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
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.<br>
<br>
</div>
</blockquote>
<div><br>
</div>
<div>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" (<a moz-do-not-send="true"
href="http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/f91c799f7bfb">http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/f91c799f7bfb</a>).
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.<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
Thanks.<br>
<br>
<br>
<blockquote
cite="mid:CA+3eh105jG=-g3czKvamJnnA_1vw5XAWM7puBKa-6caR3KAGVw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<div>
</div>
:<br>
<br>
<div><br>
</div>
<div>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.<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
Okay.<br>
<br>
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.<br>
<br>
-Alan<br>
<br>
<br>
<br>
</body>
</html>