Re: RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

2018-02-04 Thread Chris Yin
A reminder that this need a reviewer's approval, many thanks

Regards,
Chris

> On 23 Jan 2018, at 6:14 PM, Chris Yin  wrote:
> 
> Thank you Alan, I just moved it to com/sun/jndi/dns/ as you suggested and 
> removed unused "@modules jdk.naming.dns/com.sun.jndi.dns”, updated webrev as 
> below, thanks
> 
> http://cr.openjdk.java.net/~xiaofeya/8195976/webrev.00/ 
> 
> 
> Regards,
> Chris
> 
>> On 23 Jan 2018, at 3:53 PM, Alan Bateman  wrote:
>> 
>> 
>> 
>> On 23/01/2018 07:01, Chris Yin wrote:
>>> Please review the added JNDI test 
>>> javax/naming/dns/AttributeTests/GetAny.java, thanks
>>> 
>>> 
>> You may want to move it to com/sun/jndi/dns so that it's with the other 
>> tests for the DNS provider (as there is no javax.naming.dns API). Also I 
>> suspect you don't need "@modules jdk.naming.dns/com.sun.jndi.dns" as it 
>> doesn't appear to make direct use of the classes in the implementation.
>> 
>> -Alan
> 



Re: [JDK 11] RFR 8195981: Move some tests to OpenJDK for jdk_lang test group

2018-02-04 Thread Amy Lu

On 03/02/2018 2:36 AM, mandy chung wrote:


On 2/2/18 7:02 AM, Amy Lu wrote:
Please review the patch to move some tests to OpenJDK for jdk_lang 
test group.


bug: https://bugs.openjdk.java.net/browse/JDK-8195981
webrev: http://cr.openjdk.java.net/~amlu/8195981/webrev.00/



I suggest to merge jdk/vm/misc and jdk/vm/monitor directory and
rename to jdk/vm/runtime which avoids "misc" a dumping group
and is explicit that this directory is for runtime-related tests.
JITClassInit.java seems a JIT related test and maybe moved it
to jdk/vm/jit.

ExceptionInInit.java
Can you reformat the comment block line 59-95 where some lines
should be merged into the line above.


Thank you Mandy for reviewing. Pushed with changes as suggested.



Bug4404588.java can be renamed to reflect what this test does.
What about renaming it to
java/lang/Character/UnicodeBlock/SpecialsUnicodeBlock.java?


Forgot this one. Will do in separate bugid later.

Thanks,
Amy



Otherwise, looks okay.

Mandy




Re: Cannot add attribute into main attributes of a jar if there is no version

2018-02-04 Thread Philipp Kunz

duplicate of https://bugs.openjdk.java.net/browse/JDK-6910466?


On 30.01.2018 02:44, Weijun Wang wrote:



On Jan 30, 2018, at 8:57 AM, mandy chung  wrote:



On 1/29/18 4:22 PM, Weijun Wang wrote:

Ping again.



On Jan 22, 2018, at 1:12 PM, Weijun Wang 
  wrote:

src/java.base/share/classes/java/util/jar/Attributes.java:

   329  @SuppressWarnings("deprecation")
   330  void writeMain(DataOutputStream out) throws IOException
   331  {
   332  // write out the *-Version header first, if it exists
   333  String vername = Name.MANIFEST_VERSION.toString();
   334  String version = getValue(vername);
   335  if (version == null) {
   336  vername = Name.SIGNATURE_VERSION.toString();
   337  version = getValue(vername);
   338  }
   339  
   340  if (version != null) {
   341  out.writeBytes(vername+": "+version+"\r\n");
   342  }
   343  
   344  // write out all attributes except for the version
   345  // we wrote out earlier
   346  for (Entry e : entrySet()) {
   347  String name = ((Name) e.getKey()).toString();
   348  if ((version != null) && !(name.equalsIgnoreCase(vername))) 
{

So, if there is no existing MANIFEST_VERSION or SIGNATURE_VERSION, then version 
is null and the check above will be false for ever and any other attribute 
cannot be written out.

Is this intended? If so, we can exit with an else block after line 342.


 From code inspection, I agree that this method is a nop if there is no 
Manifest-Version attribute or Signature-Attribute.  This can return quickly 
without iterating the entry set.   I don't see any issue to make it an else 
block.

On the other hand, if this is not intended we should fix line 348 and remove 
the version != null check. I cannot find a place saying a MANIFEST_VERSION or 
SIGNATURE_VERSION must be provided. Even if so, this should be an error and 
it's not a good idea to silently drop all other attributes in the main section.

Anyway, I filed https://bugs.openjdk.java.net/browse/JDK-8196371.

--Max


This method is only called from Manifest::write method which does not mention 
Signature-Version but I don't have the history to tell if this is intended or 
not.

Mandy






Re: RFR 8195059: Update java.net Socket and DatagramSocket implementations to use Cleaner

2018-02-04 Thread Florian Weimer
* Roger Riggs:

> Updated in place.
>    http://cr.openjdk.java.net/~rriggs/webrev-net-cleanup-8195059/

Doesn't this leak the file descriptor of SocketCleanable.register
throws?