8034856/8034857: More gcc warnings

[Mikael Vidstedt]

On 2014-02-19 09:07, Alan Bateman wrote:
> On 18/02/2014 19:45, Mikael Vidstedt wrote:
>>
>> That makes sense, and in fact parse_manifest.c does not even appear 
>> to allow for \t, so I'm more and more starting to think that a 
>> reasonable implementation in this context would be:
>>
>> static int isNormalSpace(int c) { return c == ' '; }
>>
>> In which case it probably shouldn't even be a separate function to 
>> start with. I would like to get a second opinion on the implications 
>> of only checking for ' ' (0x20) though.
>>
>> If we want to allow both ' ' and \t we should probably call the 
>> function isblankAscii.
> Thanks again for taking this. On \t then if it's nor handled by the 
> parsing code then isNormalSpace should be fine.

Since I'm not exactly an expert on the code in question I would 
certainly appreciate it if somebody could verify me on that. I'm looking 
at parse_nv_pair (lines 430-542) in:

http://hg.openjdk.java.net/jdk9/dev/jdk/file/c766ec3e4877/src/share/bin/parse_manifest.c

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/

Cheers,
Mikael

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