+1

On 03/01/2018 11:59 AM, Roger Riggs wrote:
Hi Alan,

Thanks for the review. I added:

"There must be a colon and a SPACE after the name; the combined length will not 
exceed 72 characters."

Webrev updated in place:
   http://cr.openjdk.java.net/~rriggs/webrev-jar-6372077.patch/

Thanks, Roger


On 2/26/2018 6:50 AM, Alan Bateman wrote:
On 23/02/2018 19:58, Roger Riggs wrote:
Please review this contribution from Philipp Kunz to handle manifest attribute 
names up to 70 bytes.

The change passes the available regression tests.
Manifest handling is somewhat sensitive so an additional review is appreciated.

Webrev: (rebased from the original patch of 2/22/18)
http://cr.openjdk.java.net/~rriggs/webrev-jar-6372077.patch/

Issue:
https://bugs.openjdk.java.net/browse/JDK-6372077
I looked at the changes to the manifest code and I think they are okay. I don't 
have time to study the tests in detail.

I think it would be useful to copy the clarification note "(there must be a colon 
and a SPACE after the name)" from the JAR file spec to the equivalent sentence in 
the Attributes class description. That will help anyone wondering why the limit is 70 
rather than 72. This change shouldn't need a CSR as it's not adding any new assertions.

-Alan


Reply via email to