Re: Code Review Request: 8021418

2013-11-27 Thread Xuelei Fan
On 11/28/2013 5:38 AM, Rajan Halade wrote:
> On 11/27/2013 13:07, Bradford Wetmore wrote:
>> Where did this workaround suggestion come from?
> Refer to JDK-6978415, it has possible workaround listed.
The spec of ServerSocket.setReuseAddress() says, "When a ServerSocket is
created the initial setting of SO_REUSEADDR is not defined."  I think
it's nice to explicitly set the value in case of address reuse when
using automatically allocated server port.

Xuelei


hg: jdk8/tl/jdk: 8021418: Intermittent: SSLSocketSSLEngineTemplate.java test fails with timeout

2013-11-27 Thread jason . uh
Changeset: 5ac7cd164300
Author:juh
Date:  2013-11-27 15:25 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/5ac7cd164300

8021418: Intermittent: SSLSocketSSLEngineTemplate.java test fails with timeout
Reviewed-by: xuelei, wetmore
Contributed-by: rajan.hal...@oracle.com

! test/sun/security/ssl/templates/SSLSocketSSLEngineTemplate.java



Re: Thoughts on possible options to JDK-8027598

2013-11-27 Thread Rajan Halade


On 11/27/2013 12:47, Bradford Wetmore wrote:

> jtreg team? I am not aware of one. Jon Gibbons works on this in his
> spare time.

I would call him "the JTREG team."

Was there an ETA for this?  Balancing recent test stabilization 
efforts with this, you may consider consider adding othervm and then 
back it out when the JTREG fix is made.
I am discussing this with Jon and will soon update with summary of it. I 
am also working on adding /othervm to affected tests.


- Rajan


Brad



On 11/25/2013 2:29 AM, Chris Hegarty wrote:

On 22/11/13 21:11, Rajan Halade wrote:


On 11/22/2013 11:42, Sean Mullan wrote:

On 11/21/2013 04:53 AM, Chris Hegarty wrote:
If I am correct, JTREG has support provider cleanup already. 
But the

question is even JTREG reset the providers, it still cannot ensure
next
test won't be impacted because in some circumstances JDK may 
need to

cache something which depends on previous providers. Still need to
analysis the test case by case.


Right, any static or cached data may be invalid, and this will
require
careful changes to the JRE itself.

Pardon my ignorance - if I gather correctly then ProvidersSnapshot
library also doesn't sandbox effects completely.


FYI. I am not part of the security team, and someone from the 
security

group should ultimately have to agree/disagree, but we have similar
issues in other parts of the platform. I don't know the specifics of
the
code here so it may, or may not, be an issue.

If any part of the security framework stores values in static fields,
that is dependent on the security providers, then when the providers
change this static value may be incorrect.

We encounter this from time to time with system properties. E.g. JDK
HTTP Client code reads system property and stores in static field, 
JDK

HTTP Client will never never re-read the property as it believes it
will
not change. Having jtreg reset/clean certain system properties 
will not

help in this case.


Yes, the example with system properties is a good one.

However, providers are designed to be added and removed dynamically
(see the java.security.Security API), so if there is some static
information not being cleared when providers are reset, I would tend
to think it may be a bug in the JDK.


OK, if this is the case then that is fine.


yes. thanks!


My preference would be change jtreg to clear/restore providers, and
more thoroughly analyze any subsequent test failures as it may be a
bug in the JDK.
Ok, I will follow up with jtreg team to find out when they can 
commit to

timeline.


jtreg team? I am not aware of one. Jon Gibbons works on this in his
spare time.  You may want to take a look at jtreg [1] in the code tools
project.

-Chris.

[1] http://openjdk.java.net/projects/code-tools/jtreg/



- Rajan


--Sean







Re: Code Review Request: 8021418

2013-11-27 Thread Rajan Halade


On 11/27/2013 13:07, Bradford Wetmore wrote:

Where did this workaround suggestion come from?

Refer to JDK-6978415, it has possible workaround listed.


Please link these two issues (JDK-6978415) together.

the two bugs are linked now.


Have you talked to the network team about this?

No, since workaround suggested solved the issue.

Thanks,
Rajan


brad




On 11/26/2013 6:09 PM, Xuelei Fan wrote:

This change looks fine.

JSSE regression tests use a lot of code as "new ServerSocket(0)", we may
want a cleanup for test stabilization.

Xuelei

On 11/27/2013 9:13 AM, Rajan Halade wrote:

May I request you to review this simple change -

http://cr.openjdk.java.net/~juh/rajan/8021418/


The test is modified to set SO_REUSEADDR on ServerSocket to false for
stabilization.

Thanks,
Rajan






Re: Code Review Request: 8021418

2013-11-27 Thread Bradford Wetmore

Where did this workaround suggestion come from?

Please link these two issues (JDK-6978415) together.

Have you talked to the network team about this?

brad




On 11/26/2013 6:09 PM, Xuelei Fan wrote:

This change looks fine.

JSSE regression tests use a lot of code as "new ServerSocket(0)", we may
want a cleanup for test stabilization.

Xuelei

On 11/27/2013 9:13 AM, Rajan Halade wrote:

May I request you to review this simple change -

http://cr.openjdk.java.net/~juh/rajan/8021418/


The test is modified to set SO_REUSEADDR on ServerSocket to false for
stabilization.

Thanks,
Rajan




Re: Code Review Request: 8025763

2013-11-27 Thread Bradford Wetmore

Sean wrote:

> That kind of seems like a javadoc bug to me. Shouldn't it add the
> @since tag as part of inheriting the javadoc?

On the chance that this is a real bug, I filed this yesterday:

https://bugs.openjdk.java.net/browse/JDK-8029241

@throws/@since are both missing (others?), it seems more efficient from 
a developer perspective to simply copy this info to the overridden 
classes, instead of making the developer drill down to get the rest of 
the information.


Tony wrote:

> Let me see if setting @since doesn't cause the inheritance of the
> other tag to stop. If it doesn't I'll fix it for 8.

I'm guessing that if you add anything, it's going to suppress the 
inherited javadoc, so you'll have to wait for JDK-8029241 to be fixed.


But for the case of new stuff where you've provided new text, I feel it 
should have an @since added.


And feel free to clean up the > 80 char lines!  ;)

Brad



On 11/27/2013 12:46 PM, Anthony Scarpino wrote:



On Nov 27, 2013, at 10:12 AM, Sean Mullan  wrote:


On 11/26/2013 08:20 PM, Bradford Wetmore wrote:
Tony,

I note the @since's are missing for the new methods, both in the
generated output in the overridden methods (i.e. no javadoc), and the
methods in which you've changed the behavior (i.e. new javadoc).

I'm not sure what you can do about the previous behavior (cc'ing
Mike/Sowmya, maybe they know), but what about the new ones?


That kind of seems like a javadoc bug to me. Shouldn't it add the @since tag as 
part of inheriting the javadoc?

Anyway, adding @since tags would be considered a docs only change, so Tony you 
could still fix this for JDK 8 - can you file a bug?

Thanks,
Sean


My first thought was it would be a javadoc issue too.

Let me see if setting @since doesn't cause the inheritance of the other tag to 
stop. If it doesn't I'll fix it for 8.

Tony





Brad




On 10/21/2013 2:48 PM, Sean Mullan wrote:

On 10/18/2013 10:52 PM, Anthony Scarpino wrote:
I've updated the webrev

http://cr.openjdk.java.net/~ascarpino/8025763/webrev.01/


Update looks good.

--Sean




Re: Thoughts on possible options to JDK-8027598

2013-11-27 Thread Bradford Wetmore

> jtreg team? I am not aware of one. Jon Gibbons works on this in his
> spare time.

I would call him "the JTREG team."

Was there an ETA for this?  Balancing recent test stabilization efforts 
with this, you may consider consider adding othervm and then back it out 
when the JTREG fix is made.


Brad



On 11/25/2013 2:29 AM, Chris Hegarty wrote:

On 22/11/13 21:11, Rajan Halade wrote:


On 11/22/2013 11:42, Sean Mullan wrote:

On 11/21/2013 04:53 AM, Chris Hegarty wrote:

If I am correct, JTREG has support provider cleanup already. But the
question is even JTREG reset the providers, it still cannot ensure
next
test won't be impacted because in some circumstances JDK may need to
cache something which depends on previous providers. Still need to
analysis the test case by case.


Right, any static or cached data may be invalid, and this will
require
careful changes to the JRE itself.

Pardon my ignorance - if I gather correctly then ProvidersSnapshot
library also doesn't sandbox effects completely.


FYI. I am not part of the security team, and someone from the security
group should ultimately have to agree/disagree, but we have similar
issues in other parts of the platform. I don't know the specifics of
the
code here so it may, or may not, be an issue.

If any part of the security framework stores values in static fields,
that is dependent on the security providers, then when the providers
change this static value may be incorrect.

We encounter this from time to time with system properties. E.g. JDK
HTTP Client code reads system property and stores in static field, JDK
HTTP Client will never never re-read the property as it believes it
will
not change. Having jtreg reset/clean certain system properties will not
help in this case.


Yes, the example with system properties is a good one.

However, providers are designed to be added and removed dynamically
(see the java.security.Security API), so if there is some static
information not being cleared when providers are reset, I would tend
to think it may be a bug in the JDK.


OK, if this is the case then that is fine.


yes. thanks!


My preference would be change jtreg to clear/restore providers, and
more thoroughly analyze any subsequent test failures as it may be a
bug in the JDK.

Ok, I will follow up with jtreg team to find out when they can commit to
timeline.


jtreg team? I am not aware of one. Jon Gibbons works on this in his
spare time.  You may want to take a look at jtreg [1] in the code tools
project.

-Chris.

[1] http://openjdk.java.net/projects/code-tools/jtreg/



- Rajan


--Sean





Re: Code Review Request: 8025763

2013-11-27 Thread Anthony Scarpino

> On Nov 27, 2013, at 10:12 AM, Sean Mullan  wrote:
> 
>> On 11/26/2013 08:20 PM, Bradford Wetmore wrote:
>> Tony,
>> 
>> I note the @since's are missing for the new methods, both in the
>> generated output in the overridden methods (i.e. no javadoc), and the
>> methods in which you've changed the behavior (i.e. new javadoc).
>> 
>> I'm not sure what you can do about the previous behavior (cc'ing
>> Mike/Sowmya, maybe they know), but what about the new ones?
> 
> That kind of seems like a javadoc bug to me. Shouldn't it add the @since tag 
> as part of inheriting the javadoc?
> 
> Anyway, adding @since tags would be considered a docs only change, so Tony 
> you could still fix this for JDK 8 - can you file a bug?
> 
> Thanks,
> Sean

My first thought was it would be a javadoc issue too.

Let me see if setting @since doesn't cause the inheritance of the other tag to 
stop. If it doesn't I'll fix it for 8. 

Tony

> 
>> 
>> Brad
>> 
>> 
>> 
>>> On 10/21/2013 2:48 PM, Sean Mullan wrote:
 On 10/18/2013 10:52 PM, Anthony Scarpino wrote:
 I've updated the webrev
 
 http://cr.openjdk.java.net/~ascarpino/8025763/webrev.01/
>>> 
>>> Update looks good.
>>> 
>>> --Sean
> 


Re: Code Review Request: 8025763

2013-11-27 Thread Sean Mullan

On 11/26/2013 08:20 PM, Bradford Wetmore wrote:

Tony,

I note the @since's are missing for the new methods, both in the
generated output in the overridden methods (i.e. no javadoc), and the
methods in which you've changed the behavior (i.e. new javadoc).

I'm not sure what you can do about the previous behavior (cc'ing
Mike/Sowmya, maybe they know), but what about the new ones?


That kind of seems like a javadoc bug to me. Shouldn't it add the @since 
tag as part of inheriting the javadoc?


Anyway, adding @since tags would be considered a docs only change, so 
Tony you could still fix this for JDK 8 - can you file a bug?


Thanks,
Sean



Brad



On 10/21/2013 2:48 PM, Sean Mullan wrote:

On 10/18/2013 10:52 PM, Anthony Scarpino wrote:

I've updated the webrev

http://cr.openjdk.java.net/~ascarpino/8025763/webrev.01/


Update looks good.

--Sean




hg: jdk8/tl/jdk: 8028771: regression test java/util/Locale/LocaleProviders.sh failed

2013-11-27 Thread naoto . sato
Changeset: 2370d285d08b
Author:naoto
Date:  2013-11-27 10:01 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2370d285d08b

8028771: regression test java/util/Locale/LocaleProviders.sh failed
Reviewed-by: alanb

! test/java/util/Locale/LocaleProviders.java
! test/java/util/Locale/LocaleProviders.sh



hg: jdk8/tl/jdk: 8016839: JSR292: AME instead of IAE when calling a method

2013-11-27 Thread vladimir . kozlov
Changeset: e822676cd3cd
Author:jrose
Date:  2013-11-26 17:16 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e822676cd3cd

8016839: JSR292: AME instead of IAE when calling a method
Summary: Catch missing-because-illegal case for itable entries and use an 
exception-throwing method instead of null.
Reviewed-by: acorn, jrose, coleenp
Contributed-by: david.r.ch...@oracle.com

! src/share/classes/sun/misc/Unsafe.java



Re: [OpenJDK 2D-Dev] RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

2013-11-27 Thread Phil Race

Hi,
I see you've already received a ton of good feedback on this v2.

I have just a few  things to add.
I don't know what symlinks might exist on AIX but it seems odd to
me that you have :-

138 static char *fullAixFontPath[] = {
 139 "/usr/lpp/X11/lib/X11/fonts/Type1",
..

but the paths in the new file aix.fontconfig.properties look like this :-

/usr/lib/X11/fonts/Type1/cour.pfa


Also for the build to pick up a file called
*src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties

*it seems like you should have added a section to handle this path to
jdk/makefiles/gendata/GenDataFontConfig.gmk

That seems to be where the new build handles such files.

Are you seeing the .bfc and .src files created ?

At runtime it'll get picked up so so long as System.getProperty("os.name")
returns "aix" (all lower-case) so I think that's OK. Its the build time part
I'd expect to see but don't.

-phil.

On 11/20/2013 10:26 AM, Volker Simonis wrote:

Hi,

this is the second review round for "8024854: Basic changes and files 
to build the class library on AIX 
". The previous 
reviews can be found at the end of this mail in the references section.


I've tried to address all the comments and suggestions from the first 
round and to further streamline the patch (it perfectly builds on 
Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). 
The biggest change compared to the first review round is the new 
"aix/" subdirectory which I've now created under "jdk/src" and which 
contains AIX-only code.


The webrev is against our 
http://hg.openjdk.java.net/ppc-aix-port/stage repository which has 
been recently synchronised with the jdk8 master:


http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/ 



Below (and in the webrev) you can find some comments which explain the 
changes to each file. In order to be able to push this big change, I 
need the approval of reviewers from the lib, net, svc, 2d, awt and sec 
groups. So please don't hesitate to jump at it - there are enough 
changes for everybody:)


Thank you and best regards,
Volker

*References:*

The following reviews have been posted so far (thanks Iris for 
collecting them :)


- Alan Bateman (lib): Initial comments (16 Sep [2])
- Chris Hegarty (lib/net): Initial comments (20 Sep [3])
- Michael McMahon (net): Initial comments (20 Sept [4])
- Steffan Larsen (svc): APPROVED (20 Sept [5])
- Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments 
(15 Oct [7])

- Sean Mullan (sec): Initial comments (26 Sept [8])

[2]: 
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html
[3]: 
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html
[4]: 
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html
[5]: 
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html
[6]: 
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html
[7]: 
http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html
[8]: 
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html



*Detailed change description:*

The new "jdk/src/aix" subdirectory contains the following new and 
AIX-specific files for now:

  src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
  src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
  src/aix/classes/sun/nio/ch/AixPollPort.java
  src/aix/classes/sun/nio/fs/AixFileStore.java
  src/aix/classes/sun/nio/fs/AixFileSystem.java
  src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
  src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
  src/aix/classes/sun/tools/attach/AixAttachProvider.java
  src/aix/classes/sun/tools/attach/AixVirtualMachine.java
  src/aix/native/java/net/aix_close.c
  src/aix/native/sun/nio/ch/AixPollPort.c
  src/aix/native/sun/nio/fs/AixNativeDispatcher.c
  src/aix/native/sun/tools/attach/AixVirtualMachine.c
  src/aix/porting/porting_aix.c
  src/aix/porting/porting_aix.h


src/aix/porting/porting_aix.c
src/aix/porting/porting_aix.h

  * Added these two files for AIX relevant utility code.
  * Currently these files only contain an implementation of |dladdr|
which is not available on AIX.
  * In the first review round the existing |dladdr| users in the code
either called the version from the HotSpot on AIX
(|src/solaris/native/sun/awt/awt_LoadLibrary.c|) or had a private
copy of the whole implementation
(|src/solaris/demo/jvmti/hprof/hprof_md.c|). This is now not
necessary any more.

The new file layout required some small changes to the makefiles to 
make them aware of the new directory locations:



makefiles/CompileDemos.gmk

  * Add an extra argument to |SetupJVMTIDemo| which can be used to
pass additional source locations.
  * Currently this is only used on AIX for the AIX p

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

2013-11-27 Thread Volker Simonis
Hi Artem,

thanks a lot for your review. Please find my comments inline:

On Monday, November 25, 2013, Artem Ananiev wrote:

> Hi, Volker,
>
> just a few very minor comments about the client changes:
>
> 1. mlib_sys.c: the change is fine, but it makes the following comment
> obsolete.


You're right. I'll update the comment and try to find a
corresponding  reference for the AIX compiler.


> 2. XRBackendNative.c: the same comment here.


Again, I'll update the comment accordingly.


> 3. Awt2dLibraries.gmk: $(JDK_TOPDIR)/src/aix/porting/porting_aix.c would
> be better than just porting_aix.c


Good catch! I think the right solution would be to add
$(JDK_TOPDIR)/src/aix/porting to LIBAWT_DIRS. This will also add the
directory automatically to LIBAWT_CFLAGS.

I'll prepare a final webrev with all the proposed changes tomorrow.

Thanks once again,
Volker


>
> Thanks,
>
> Artem
>
> On 11/20/2013 10:26 PM, Volker Simonis wrote:
>
>> Hi,
>>
>> this is the second review round for "8024854: Basic changes and files to
>> build the class library on AIX
>> ". The previous
>> reviews can be found at the end of this mail in the references section.
>>
>> I've tried to address all the comments and suggestions from the first
>> round and to further streamline the patch (it perfectly builds on
>> Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). The
>> biggest change compared to the first review round is the new "aix/"
>> subdirectory which I've now created under "jdk/src" and which contains
>> AIX-only code.
>>
>> The webrev is against our http://hg.openjdk.java.net/ppc-aix-port/stage
>> repository which has been recently synchronised with the jdk8 master:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/
>>
>> Below (and in the webrev) you can find some comments which explain the
>> changes to each file. In order to be able to push this big change, I
>> need the approval of reviewers from the lib, net, svc, 2d, awt and sec
>> groups. So please don't hesitate to jump at it - there are enough
>> changes for everybody:)
>>
>> Thank you and best regards,
>> Volker
>>
>> *References:*
>>
>> The following reviews have been posted so far (thanks Iris for
>> collecting them :)
>>
>> - Alan Bateman (lib): Initial comments (16 Sep [2])
>> - Chris Hegarty (lib/net): Initial comments (20 Sep [3])
>> - Michael McMahon (net): Initial comments (20 Sept [4])
>> - Steffan Larsen (svc): APPROVED (20 Sept [5])
>> - Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments
>> (15 Oct [7])
>> - Sean Mullan (sec): Initial comments (26 Sept [8])
>>
>> [2]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001045.html
>> [3]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001078.html
>> [4]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001079.html
>> [5]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001077.html
>> [6]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001069.html
>> [7]: http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/
>> 003826.html
>> [8]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001081.html
>>
>>
>> *Detailed change description:*
>>
>> The new "jdk/src/aix" subdirectory contains the following new and
>> AIX-specific files for now:
>>
>>   src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
>>   src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
>>   src/aix/classes/sun/nio/ch/AixPollPort.java
>>   src/aix/classes/sun/nio/fs/AixFileStore.java
>>   src/aix/classes/sun/nio/fs/AixFileSystem.java
>>   src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
>>   src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
>>   src/aix/classes/sun/tools/attach/AixAttachProvider.java
>>   src/aix/classes/sun/tools/attach/AixVirtualMachine.java
>>   src/aix/native/java/net/aix_close.c
>>   src/aix/native/sun/nio/ch/AixPollPort.c
>>   src/aix/native/sun/nio/fs/AixNativeDispatcher.c
>>   src/aix/native/sun/tools/attach/AixVirtualMachine.c
>>   src/aix/porting/porting_aix.c
>>   src/aix/porting/porting_aix.h
>>
>>
>> src/aix/porting/porting_aix.c
>> src/aix/porting/porting_aix.h
>>
>>   * Added these two files for AIX relevant utility code.
>>   * Currently these files only contain an implementation of |dladdr|
>> which is not available on AIX.
>>   * In the first review round the existing |dladdr| users in the code
>> either called the version from the HotSpot on AIX
>> (|src/solaris/native/sun/awt/awt_LoadLibrary.c|) or had a private
>> copy of the whole implementation
>> (|src/solaris/demo/jvmti/hprof/hprof_md.c|). This is now not
>> necessary any more.
>>
>> The new file layout required some small changes to the makefiles to make
>> them aware of the new directory locations:
>>
>>
>> mak

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

2013-11-27 Thread Volker Simonis
We've synced our staging repository yesterday with the latest jdk8-b117 and
I noticed that change "8025985: com.sun.management.OSMBeanFactory should
not be public" moved the file src/solaris/native/com/sun/
management/UnixOperatingSystem_md.c to
src/solaris/native/sun/management/OperatingSystemImpl.c. Fortunately, my
changes to UnixOperatingSystem_md.c described in the webrev apply cleanly
to the new file (I've tested this locally).

I'll update the webrev accordingly once I've collected some more feedback.

Thank you and best regards,
Volker


On Wed, Nov 20, 2013 at 7:26 PM, Volker Simonis wrote:

> Hi,
>
> this is the second review round for "8024854: Basic changes and files to
> build the class library on 
> AIX".
> The previous reviews can be found at the end of this mail in the references
> section.
>
> I've tried to address all the comments and suggestions from the first
> round and to further streamline the patch (it perfectly builds on
> Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). The
> biggest change compared to the first review round is the new "aix/"
> subdirectory which I've now created under "jdk/src" and which contains
> AIX-only code.
>
> The webrev is against our 
> http://hg.openjdk.java.net/ppc-aix-port/stagerepository which has been 
> recently synchronised with the jdk8 master:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/
>
> Below (and in the webrev) you can find some comments which explain the
> changes to each file. In order to be able to push this big change, I need
> the approval of reviewers from the lib, net, svc, 2d, awt and sec groups.
> So please don't hesitate to jump at it - there are enough changes for
> everybody:)
>
> Thank you and best regards,
> Volker
>
> *References:*
>
> The following reviews have been posted so far (thanks Iris for collecting
> them :)
>
> - Alan Bateman (lib): Initial comments (16 Sep [2])
> - Chris Hegarty (lib/net): Initial comments (20 Sep [3])
> - Michael McMahon (net): Initial comments (20 Sept [4])
> - Steffan Larsen (svc): APPROVED (20 Sept [5])
> - Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments
> (15 Oct [7])
> - Sean Mullan (sec): Initial comments (26 Sept [8])
>
> [2]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html
> [3]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html
> [4]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html
> [5]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html
> [6]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html
> [7]:
> http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html
> [8]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html
>
>
> *Detailed change description:*
>
> The new "jdk/src/aix" subdirectory contains the following new and
> AIX-specific files for now:
>
>  src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
>  src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
>  src/aix/classes/sun/nio/ch/AixPollPort.java
>  src/aix/classes/sun/nio/fs/AixFileStore.java
>  src/aix/classes/sun/nio/fs/AixFileSystem.java
>  src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
>  src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
>  src/aix/classes/sun/tools/attach/AixAttachProvider.java
>  src/aix/classes/sun/tools/attach/AixVirtualMachine.java
>  src/aix/native/java/net/aix_close.c
>  src/aix/native/sun/nio/ch/AixPollPort.c
>  src/aix/native/sun/nio/fs/AixNativeDispatcher.c
>  src/aix/native/sun/tools/attach/AixVirtualMachine.c
>  src/aix/porting/porting_aix.c
>  src/aix/porting/porting_aix.h
>
> src/aix/porting/porting_aix.c
> src/aix/porting/porting_aix.h
>
>- Added these two files for AIX relevant utility code.
>- Currently these files only contain an implementation of dladdr which
>is not available on AIX.
>- In the first review round the existing dladdr users in the code
>either called the version from the HotSpot on AIX (
>src/solaris/native/sun/awt/awt_LoadLibrary.c) or had a private copy of
>the whole implementation (src/solaris/demo/jvmti/hprof/hprof_md.c).
>This is now not necessary any more.
>
>  The new file layout required some small changes to the makefiles to make
> them aware of the new directory locations:
>
> makefiles/CompileDemos.gmk
>
>- Add an extra argument to SetupJVMTIDemo which can be used to pass
>additional source locations.
>- Currently this is only used on AIX for the AIX porting utilities
>which are required by hprof.
>
> makefiles/lib/Awt2dLibraries.gmk
> makefiles/lib/ServiceabilityLibraries.gmk
>
>- On AIX add the sources of the AIX porting utilities to the build.
>They are required by src/solaris/native/sun/awt/awt_LoadLibrary.c and
>src/solaris/demo/jvmti/hprof/hpr

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

2013-11-27 Thread Volker Simonis
Hi Sean,

thanks a lot for you review.

Please let me know once you start working on 6997010 so I can update the
corresponding AIX file accordingly.

Regards,
Volker

On Monday, November 25, 2013, Sean Mullan wrote:

> Hi Volker,
>
> The security changes look fine. I'm not crazy that we now have to maintain
> one additional java.security file which is the exact same as
> java.security-linux, but this is really an existing issue with duplicated
> content across the java.security files which I will try to fix early in JDK
> 9: https://bugs.openjdk.java.net/browse/JDK-6997010
>
> Thanks,
> Sean
>
> On 11/20/2013 01:26 PM, Volker Simonis wrote:
>
>> Hi,
>>
>> this is the second review round for "8024854: Basic changes and files to
>> build the class library on
>> AIX".
>> The previous reviews can be found at the end of this mail in the
>> references
>> section.
>>
>> I've tried to address all the comments and suggestions from the first
>> round
>> and to further streamline the patch (it perfectly builds on Linux/x86_64,
>> Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). The biggest change
>> compared to the first review round is the new "aix/" subdirectory which
>> I've now created under "jdk/src" and which contains AIX-only code.
>>
>> The webrev is against our
>> http://hg.openjdk.java.net/ppc-aix-port/stagerepository which has been
>> recently synchronised with the jdk8 master:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/
>>
>> Below (and in the webrev) you can find some comments which explain the
>> changes to each file. In order to be able to push this big change, I need
>> the approval of reviewers from the lib, net, svc, 2d, awt and sec groups.
>> So please don't hesitate to jump at it - there are enough changes for
>> everybody:)
>>
>> Thank you and best regards,
>> Volker
>>
>> *References:*
>>
>> The following reviews have been posted so far (thanks Iris for collecting
>> them :)
>>
>> - Alan Bateman (lib): Initial comments (16 Sep [2])
>> - Chris Hegarty (lib/net): Initial comments (20 Sep [3])
>> - Michael McMahon (net): Initial comments (20 Sept [4])
>> - Steffan Larsen (svc): APPROVED (20 Sept [5])
>> - Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments (15
>> Oct [7])
>> - Sean Mullan (sec): Initial comments (26 Sept [8])
>>
>> [2]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001045.html
>> [3]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001078.html
>> [4]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001079.html
>> [5]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001077.html
>> [6]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001069.html
>> [7]: http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/
>> 003826.html
>> [8]:
>> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001081.html
>>
>>
>> *Detailed change description:*
>>
>> The new "jdk/src/aix" subdirectory contains the following new and
>> AIX-specific files for now:
>>
>>   src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
>>   src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
>>   src/aix/classes/sun/nio/ch/AixPollPort.java
>>   src/aix/classes/sun/nio/fs/AixFileStore.java
>>   src/aix/classes/sun/nio/fs/AixFileSystem.java
>>   src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
>>   src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
>>   src/aix/classes/sun/tools/attach/AixAttachProvider.java
>>   src/aix/classes/sun/tools/attach/AixVirtualMachine.java
>>   src/aix/native/java/net/aix_close.c
>>   src/aix/native/sun/nio/ch/AixPollPort.c
>>   src/aix/native/sun/nio/fs/AixNativeDispatcher.c
>>   src/aix/native/sun/tools/attach/AixVirtualMachine.c
>>   src/aix/porting/porting_aix.c
>>   src/aix/porting/porting_aix.h
>>
>> src/aix/porting/porting_aix.c
>> src/aix/porting/porting_aix.h
>>
>> - Added these two files for AIX relevant utility code.
>> - Currently these files only contain an implementation of dladdr which
>> is not available on AIX.
>> - In the first review round the existing dladdr users in the code
>> either
>> called the version from the HotSpot on AIX (
>> src/solaris/native/sun/awt/awt_LoadLibrary.c) or had a private copy
>> of
>> the whole implementation (src/solaris/demo/jvmti/hprof/hprof_md.c).
>> This
>> is now not necessary any more.
>>
>>   The new file layout required some small changes to the makefiles to make
>> them aware of the new directory locations:
>>
>> makefiles/CompileDemos.gmk
>>
>> - Add an extra argument to SetupJVMTIDemo which can be used to pass
>> additional source locations.
>> - Currently this is only used on AIX for the AIX porting utilities
>> which
>> are required by hprof.
>>
>> makefiles/lib/Awt2dLibraries.gmk
>> makefiles/lib/Se

RE: [OpenJDK 2D-Dev] RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

2013-11-27 Thread Iris Clark
> So overall it looks good to me and should be pushed to the staging > forest 
> once you hear from others that commented previously.

I think that means Chris Hegarty, Michael McMahon, and Sergey Bylokhov.  Alan, 
please correct me if I'm wrong.

Thanks,
iris

-Original Message-
From: Alan Bateman 
Sent: Tuesday, November 26, 2013 9:03 AM
To: Volker Simonis
Cc: Vladimir Kozlov; 2d-...@openjdk.java.net; 
serviceability-...@openjdk.java.net; security-dev; 
ppc-aix-port-...@openjdk.java.net; awt-...@openjdk.java.net; Java Core Libs; 
net-dev
Subject: Re: [OpenJDK 2D-Dev] RFR(L) - 2nd round: 8024854: Basic changes and 
files to build the class library on AIX

On 26/11/2013 16:23, Volker Simonis wrote:
> Hi,
>
> thanks to everybody for the prompt and helpful reviews. Here comes the 
> final webrev which incorporates all the corrections and suggestions 
> from the second review round:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/
>
> I've successfully build (and run some smoke tests) with these changes 
> on Linux (x86_32, x86_64, ppc64), Solaris/sparcv9, Windows/x86_64, 
> MacOSX and AIX (5.3, 7.1).
>
I've skimmed over the last webrev with focus on:

NetworkingLibraries.gmk where I see this is now fixed for all platforms.

net_util.* and the platform specific net_util_md.* where I see you've added 
platformInit so it's much cleaner.

UnixNativeDispatcher.c where the error translation is now removed (and looks 
fine).

So overall it looks good to me and should be pushed to the staging forest once 
you hear from others that commented previously.

-Alan


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

2013-11-27 Thread 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  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/95399/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
>
>
>


Re: [OpenJDK 2D-Dev] RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

2013-11-27 Thread Artem Ananiev

Hi, Volker,

just a few very minor comments about the client changes:

1. mlib_sys.c: the change is fine, but it makes the following comment 
obsolete.


2. XRBackendNative.c: the same comment here.

3. Awt2dLibraries.gmk: $(JDK_TOPDIR)/src/aix/porting/porting_aix.c would 
be better than just porting_aix.c


Thanks,

Artem

On 11/20/2013 10:26 PM, Volker Simonis wrote:

Hi,

this is the second review round for "8024854: Basic changes and files to
build the class library on AIX
". The previous
reviews can be found at the end of this mail in the references section.

I've tried to address all the comments and suggestions from the first
round and to further streamline the patch (it perfectly builds on
Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). The
biggest change compared to the first review round is the new "aix/"
subdirectory which I've now created under "jdk/src" and which contains
AIX-only code.

The webrev is against our http://hg.openjdk.java.net/ppc-aix-port/stage
repository which has been recently synchronised with the jdk8 master:

http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/

Below (and in the webrev) you can find some comments which explain the
changes to each file. In order to be able to push this big change, I
need the approval of reviewers from the lib, net, svc, 2d, awt and sec
groups. So please don't hesitate to jump at it - there are enough
changes for everybody:)

Thank you and best regards,
Volker

*References:*

The following reviews have been posted so far (thanks Iris for
collecting them :)

- Alan Bateman (lib): Initial comments (16 Sep [2])
- Chris Hegarty (lib/net): Initial comments (20 Sep [3])
- Michael McMahon (net): Initial comments (20 Sept [4])
- Steffan Larsen (svc): APPROVED (20 Sept [5])
- Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments
(15 Oct [7])
- Sean Mullan (sec): Initial comments (26 Sept [8])

[2]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html
[3]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html
[4]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html
[5]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html
[6]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html
[7]: http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html
[8]:
http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html


*Detailed change description:*

The new "jdk/src/aix" subdirectory contains the following new and
AIX-specific files for now:

  src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
  src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
  src/aix/classes/sun/nio/ch/AixPollPort.java
  src/aix/classes/sun/nio/fs/AixFileStore.java
  src/aix/classes/sun/nio/fs/AixFileSystem.java
  src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
  src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
  src/aix/classes/sun/tools/attach/AixAttachProvider.java
  src/aix/classes/sun/tools/attach/AixVirtualMachine.java
  src/aix/native/java/net/aix_close.c
  src/aix/native/sun/nio/ch/AixPollPort.c
  src/aix/native/sun/nio/fs/AixNativeDispatcher.c
  src/aix/native/sun/tools/attach/AixVirtualMachine.c
  src/aix/porting/porting_aix.c
  src/aix/porting/porting_aix.h


src/aix/porting/porting_aix.c
src/aix/porting/porting_aix.h

  * Added these two files for AIX relevant utility code.
  * Currently these files only contain an implementation of |dladdr|
which is not available on AIX.
  * In the first review round the existing |dladdr| users in the code
either called the version from the HotSpot on AIX
(|src/solaris/native/sun/awt/awt_LoadLibrary.c|) or had a private
copy of the whole implementation
(|src/solaris/demo/jvmti/hprof/hprof_md.c|). This is now not
necessary any more.

The new file layout required some small changes to the makefiles to make
them aware of the new directory locations:


makefiles/CompileDemos.gmk

  * Add an extra argument to |SetupJVMTIDemo| which can be used to pass
additional source locations.
  * Currently this is only used on AIX for the AIX porting utilities
which are required by hprof.


makefiles/lib/Awt2dLibraries.gmk
makefiles/lib/ServiceabilityLibraries.gmk

  * On AIX add the sources of the AIX porting utilities to the build.
They are required by |src/solaris/native/sun/awt/awt_LoadLibrary.c|
and |src/solaris/demo/jvmti/hprof/hprof_md.c| because |dladdr| is
not available on AIX.


makefiles/lib/NioLibraries.gmk

  * Make the AIX-build aware of the new NIO source locations under
|src/aix/native/sun/nio/|.


makefiles/lib/NetworkingLibraries.gmk

  * Make the AIX-build aware of the new |aix_close.c| source location
under |src/aix/native/java/net/|.


hg: jdk8/tl/jaxp: 2 new changesets

2013-11-27 Thread michael . fang
Changeset: abd44ea60dbe
Author:mfang
Date:  2013-11-21 15:43 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/abd44ea60dbe

8028803: jdk8 l10n resource file translation update 5 - jaxp repo
Reviewed-by: joehw, yhuang

! src/com/sun/org/apache/xalan/internal/res/XSLTErrorResources_zh_CN.java
! src/com/sun/org/apache/xalan/internal/xsltc/runtime/ErrorMessages_zh_CN.java
! 
src/com/sun/org/apache/xerces/internal/impl/msg/JAXPValidationMessages_de.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_de.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_es.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_fr.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_it.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_ja.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_ko.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_pt_BR.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_sv.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_zh_CN.properties
! src/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_zh_TW.properties
! 
src/com/sun/org/apache/xerces/internal/impl/msg/XMLSchemaMessages_it.properties
! src/com/sun/org/apache/xml/internal/res/XMLErrorResources_zh_CN.java
! src/com/sun/org/apache/xpath/internal/res/XPATHErrorResources_zh_CN.java

Changeset: e65463c785ed
Author:mfang
Date:  2013-11-25 14:14 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jaxp/rev/e65463c785ed

Merge




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

2013-11-27 Thread Volker Simonis
Hi,

thanks to everybody for the prompt and helpful reviews. Here comes the
final webrev which incorporates all the corrections and suggestions from
the second review round:

http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/

I've successfully build (and run some smoke tests) with these changes on
Linux (x86_32, x86_64, ppc64), Solaris/sparcv9, Windows/x86_64, MacOSX and
AIX (5.3, 7.1).

So if nobody vetoes these changes are ready for push into our staging
repository.

@Vladimir: can I push them or do you want to run them trough JPRT?

Thank you and best regards,
Volker

PS: compared to the last webrev (
http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/), I've slightly
changed the following files:

makefiles/lib/Awt2dLibraries.gmk
makefiles/lib/NetworkingLibraries.gmk
makefiles/Setup.gmk
src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
src/aix/classes/sun/nio/ch/AixPollPort.java
src/aix/classes/sun/nio/fs/AixFileSystem.java
src/aix/native/java/net/aix_close.c
src/aix/porting/porting_aix.c
src/share/native/java/net/net_util.c
src/share/native/java/net/net_util.h
src/share/native/sun/awt/medialib/mlib_sys.c
src/solaris/bin/java_md_solinux.c
src/solaris/classes/sun/nio/ch/Port.java
src/solaris/native/java/net/net_util_md.c
src/solaris/native/sun/java2d/x11/XRBackendNative.c
src/solaris/native/sun/management/OperatingSystemImpl.c
src/solaris/native/sun/nio/fs/UnixNativeDispatcher.c
src/windows/native/java/net/net_util_md.c

Most of the changes are cosmetic, except the ones in:

src/share/native/java/net/net_util.c
src/share/native/java/net/net_util.h
src/solaris/native/java/net/net_util_md.c
src/windows/native/java/net/net_util_md.c

where I renamed 'initLocalAddrTable()' to 'platformInit()'. For AIX, this
method will now call 'aix_close_init()' as suggested by Alan.

The changes to src/solaris/native/com/sun/
management/UnixOperatingSystem_md.c are now in
src/solaris/native/sun/management/OperatingSystemImpl.c because that file
was moved by an upstream change.



On Wed, Nov 20, 2013 at 7:26 PM, Volker Simonis wrote:

> Hi,
>
> this is the second review round for "8024854: Basic changes and files to
> build the class library on 
> AIX".
> The previous reviews can be found at the end of this mail in the references
> section.
>
> I've tried to address all the comments and suggestions from the first
> round and to further streamline the patch (it perfectly builds on
> Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). The
> biggest change compared to the first review round is the new "aix/"
> subdirectory which I've now created under "jdk/src" and which contains
> AIX-only code.
>
> The webrev is against our 
> http://hg.openjdk.java.net/ppc-aix-port/stagerepository which has been 
> recently synchronised with the jdk8 master:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/
>
> Below (and in the webrev) you can find some comments which explain the
> changes to each file. In order to be able to push this big change, I need
> the approval of reviewers from the lib, net, svc, 2d, awt and sec groups.
> So please don't hesitate to jump at it - there are enough changes for
> everybody:)
>
> Thank you and best regards,
> Volker
>
> *References:*
>
> The following reviews have been posted so far (thanks Iris for collecting
> them :)
>
> - Alan Bateman (lib): Initial comments (16 Sep [2])
> - Chris Hegarty (lib/net): Initial comments (20 Sep [3])
> - Michael McMahon (net): Initial comments (20 Sept [4])
> - Steffan Larsen (svc): APPROVED (20 Sept [5])
> - Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments
> (15 Oct [7])
> - Sean Mullan (sec): Initial comments (26 Sept [8])
>
> [2]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html
> [3]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html
> [4]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html
> [5]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html
> [6]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html
> [7]:
> http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html
> [8]:
> http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html
>
>
> *Detailed change description:*
>
> The new "jdk/src/aix" subdirectory contains the following new and
> AIX-specific files for now:
>
>  src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
>  src/aix/classes/sun/nio/ch/AixAsynchronousChannelProvider.java
>  src/aix/classes/sun/nio/ch/AixPollPort.java
>  src/aix/classes/sun/nio/fs/AixFileStore.java
>  src/aix/classes/sun/nio/fs/AixFileSystem.java
>  src/aix/classes/sun/nio/fs/AixFileSystemProvider.java
>  src/aix/classes/sun/nio/fs/AixNativeDispatcher.java
>  src/aix/classes/sun/tools/attach/AixAttachProvider.java
>  src/aix/classes/sun/tools/atta

Re: [OpenJDK 2D-Dev] RFR(L) - 2nd round: 8024854: Basic changes and files to build the class library on AIX

2013-11-27 Thread Volker Simonis
Hi Phil,

thanks a lot for the review. Please find my comments inline:


On Tue, Nov 26, 2013 at 12:24 AM, Phil Race  wrote:

> Hi,
> I see you've already received a ton of good feedback on this v2.
>
> I have just a few  things to add.
> I don't know what symlinks might exist on AIX but it seems odd to
> me that you have :-
>
> 138 static char *fullAixFontPath[] = {
>  139 "/usr/lpp/X11/lib/X11/fonts/Type1",
> ..
>
> but the paths in the new file aix.fontconfig.properties look like this :-
>
> /usr/lib/X11/fonts/Type1/cour.pfa
>
>
You're absolutely right. I've updated 'aix.fontconfig.properties' to
contain the same absolute path like 'fontpath.c'. I've also added a comment
which describes to which AIX-package the fonts belong to and to which
locations they are sym-linked to.

Also for the build to pick up a file called
> *src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
>
> *it seems like you should have added a section to handle this path to
> jdk/makefiles/gendata/GenDataFontConfig.gmk
>
> That seems to be where the new build handles such files.
>
> Are you seeing the .bfc and .src files created ?
>
>
You're right. But that was already added by the general AIX-build change
"8024900: PPC64: Enable new build on AIX (jdk part)" (
http://hg.openjdk.java.net/ppc-aix-port/stage/jdk/diff/3ac08cd5e2e8/makefiles/GendataFontConfig.gmk
).

And yes, the .bfc and .src files are created and copied to the right places.


> At runtime it'll get picked up so so long as System.getProperty("os.name")
> returns "aix" (all lower-case) so I think that's OK. Its the build time
> part
> I'd expect to see but don't.
>
>
I did split the AIX change into several changes and the build changes have
been reviewed separately:

8024265: PPC64: Enable new build on AIX (
https://bugs.openjdk.java.net/browse/JDK-8024265)
8024900: PPC64: Enable new build on AIX (jdk part) (
https://bugs.openjdk.java.net/browse/JDK-8024900)

This change only contains the additional make changes which became
necessary after I started to move AIX-specific files into their own
jdk/src/aix/ directory. Everything else is already in place.

I'll prepare and test a finaly webrev with all the changes from this review
round today.

Thanks a lot,
Volker



> -phil.
>
>
> On 11/20/2013 10:26 AM, Volker Simonis wrote:
>
>> Hi,
>>
>> this is the second review round for "8024854: Basic changes and files to
>> build the class library on AIX > net/browse/JDK-8024854>". The previous reviews can be found at the end
>> of this mail in the references section.
>>
>>
>> I've tried to address all the comments and suggestions from the first
>> round and to further streamline the patch (it perfectly builds on
>> Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and Windows/x86_64). The
>> biggest change compared to the first review round is the new "aix/"
>> subdirectory which I've now created under "jdk/src" and which contains
>> AIX-only code.
>>
>> The webrev is against our 
>> http://hg.openjdk.java.net/ppc-aix-port/stagerepository which has been 
>> recently synchronised with the jdk8 master:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/ <
>> http://cr.openjdk.java.net/%7Esimonis/webrevs/8024854.v2/>
>>
>>
>> Below (and in the webrev) you can find some comments which explain the
>> changes to each file. In order to be able to push this big change, I need
>> the approval of reviewers from the lib, net, svc, 2d, awt and sec groups.
>> So please don't hesitate to jump at it - there are enough changes for
>> everybody:)
>>
>> Thank you and best regards,
>> Volker
>>
>> *References:*
>>
>>
>> The following reviews have been posted so far (thanks Iris for collecting
>> them :)
>>
>> - Alan Bateman (lib): Initial comments (16 Sep [2])
>> - Chris Hegarty (lib/net): Initial comments (20 Sep [3])
>> - Michael McMahon (net): Initial comments (20 Sept [4])
>> - Steffan Larsen (svc): APPROVED (20 Sept [5])
>> - Phil Race (2d): Initial comments  (18 Sept [6]); Additional comments
>> (15 Oct [7])
>> - Sean Mullan (sec): Initial comments (26 Sept [8])
>>
>> [2]: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001045.html
>> [3]: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001078.html
>> [4]: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001079.html
>> [5]: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001077.html
>> [6]: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001069.html
>> [7]: http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/
>> 003826.html
>> [8]: http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/
>> 2013-September/001081.html
>>
>>
>> *Detailed change description:*
>>
>>
>> The new "jdk/src/aix" subdirectory contains the following new and
>> AIX-specific files for now:
>>   src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
>>   src/aix/classes/sun/nio/ch

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

2013-11-27 Thread Phil Race
Looking only at what you needed to change this time round, all seems 
fine now.


-phil.

On 11/26/13 8:23 AM, Volker Simonis wrote:

Hi,

thanks to everybody for the prompt and helpful reviews. Here comes the 
final webrev which incorporates all the corrections and suggestions 
from the second review round:


http://cr.openjdk.java.net/~simonis/webrevs/8024854.v3/ 



I've successfully build (and run some smoke tests) with these changes 
on Linux (x86_32, x86_64, ppc64), Solaris/sparcv9, Windows/x86_64, 
MacOSX and AIX (5.3, 7.1).


So if nobody vetoes these changes are ready for push into our staging 
repository.


@Vladimir: can I push them or do you want to run them trough JPRT?

Thank you and best regards,
Volker

PS: compared to the last webrev 
(http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/ 
), I've 
slightly changed the following files:


makefiles/lib/Awt2dLibraries.gmk
makefiles/lib/NetworkingLibraries.gmk
makefiles/Setup.gmk
src/aix/classes/sun/awt/fontconfigs/aix.fontconfig.properties
src/aix/classes/sun/nio/ch/AixPollPort.java
src/aix/classes/sun/nio/fs/AixFileSystem.java
src/aix/native/java/net/aix_close.c
src/aix/porting/porting_aix.c
src/share/native/java/net/net_util.c
src/share/native/java/net/net_util.h
src/share/native/sun/awt/medialib/mlib_sys.c
src/solaris/bin/java_md_solinux.c
src/solaris/classes/sun/nio/ch/Port.java
src/solaris/native/java/net/net_util_md.c
src/solaris/native/sun/java2d/x11/XRBackendNative.c
src/solaris/native/sun/management/OperatingSystemImpl.c
src/solaris/native/sun/nio/fs/UnixNativeDispatcher.c
src/windows/native/java/net/net_util_md.c

Most of the changes are cosmetic, except the ones in:

src/share/native/java/net/net_util.c
src/share/native/java/net/net_util.h
src/solaris/native/java/net/net_util_md.c
src/windows/native/java/net/net_util_md.c

where I renamed 'initLocalAddrTable()' to 'platformInit()'. For AIX, 
this method will now call 'aix_close_init()' as suggested by Alan.


The changes to src/solaris/native/com/sun/ 
management/UnixOperatingSystem_md.c are now in 
src/solaris/native/sun/management/OperatingSystemImpl.c because that 
file was moved by an upstream change.




On Wed, Nov 20, 2013 at 7:26 PM, Volker Simonis 
mailto:volker.simo...@gmail.com>> wrote:


Hi,

this is the second review round for "8024854: Basic changes and
files to build the class library on AIX
". The previous
reviews can be found at the end of this mail in the references
section.

I've tried to address all the comments and suggestions from the
first round and to further streamline the patch (it perfectly
builds on Linux/x86_64, Linux/PPC664, AIX, Solaris/SPARC and
Windows/x86_64). The biggest change compared to the first review
round is the new "aix/" subdirectory which I've now created under
"jdk/src" and which contains AIX-only code.

The webrev is against our
http://hg.openjdk.java.net/ppc-aix-port/stage repository which has
been recently synchronised with the jdk8 master:

http://cr.openjdk.java.net/~simonis/webrevs/8024854.v2/


Below (and in the webrev) you can find some comments which explain
the changes to each file. In order to be able to push this big
change, I need the approval of reviewers from the lib, net, svc,
2d, awt and sec groups. So please don't hesitate to jump at it -
there are enough changes for everybody:)

Thank you and best regards,
Volker

*References:*

The following reviews have been posted so far (thanks Iris for
collecting them :)

- Alan Bateman (lib): Initial comments (16 Sep [2])
- Chris Hegarty (lib/net): Initial comments (20 Sep [3])
- Michael McMahon (net): Initial comments (20 Sept [4])
- Steffan Larsen (svc): APPROVED (20 Sept [5])
- Phil Race (2d): Initial comments  (18 Sept [6]); Additional
comments (15 Oct [7])
- Sean Mullan (sec): Initial comments (26 Sept [8])

[2]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001045.html
[3]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001078.html
[4]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001079.html
[5]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001077.html
[6]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001069.html
[7]:
http://mail.openjdk.java.net/pipermail/2d-dev/2013-October/003826.html
[8]:

http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/2013-September/001081.html


*Detailed change description:*

The new "jdk/src/aix" subdirectory contains the following new and
AIX-specific files for now:

  src/aix/c