8034856/8034857: More gcc warnings

[Mikael Vidstedt]

On 2014-02-23 01:19, Alan Bateman wrote:
> On 19/02/2014 18:22, Mikael Vidstedt wrote:
>> :
>>
>> The documented grammar in the comment only mentions "SPACE" and the 
>> code below doesn't make any references to \t. As a matter of fact, it 
>> only checks for one single, mandatory SPACE after the colon (enforced 
>> at line 535-536) and doesn't care to remove any space characters at 
>> the end of the value. The while loop only deals with continuations. 
>> If additional spaces do exist they will as far as I can tell be part 
>> of the value. Are they trimmed later? I'm assuming it would be nice 
>> to have both parsers (parse_manifest & JarFacade) behave the same way?
>>
>> Here's what it would look like to only check for space, but still eat 
>> any additional spaces which doesn't match what 
>> parse_manifest/parse_nv_pair does:
>>
>> http://cr.openjdk.java.net/~mikael/webrevs/isspace/webrev.01/webrev/
>>
> Sorry for the delay getting back to you on this (I've been busy with 
> other things).
>
> I checked the JAR File Specification, which is turn references RFC 822 
> as the "inspiration" for the name-value pairs. The SPACE token is just 
> ASCII SP. So I agree it's just ASCII SP that needs to be handled here, 
> not LWSP-char which includes ASCII HT.
>
> Looking at JDK-6274276 then the trimming was done to avoid 
> hard-to-diagnose problems with leading/trailing spaces. It's possible 
> that this is inconsistent with other areas where JAR file attributes 
> are used. I would suggest leaving it as is for now as this is 
> potentially changing behavior in several areas.

That sounds reasonable. I'll keep the webrev.01 approach - only check 
for and trim ASCII SP.

Thanks for the review!

Cheers,
Mikael

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140224/cc0a7399/attachment.html