8034856/8034857: More gcc warnings

[Mikael Vidstedt]

On 2014-02-18 00:33, Alan Bateman wrote:
> On 18/02/2014 03:59, Mikael Vidstedt wrote:
>>
>> How about:
>>
>> http://cr.openjdk.java.net/~mikael/webrevs/isspace/webrev.00/webrev/
>>
>> Cheers,
>> Mikael
>>
> I checked the java.lang.instrument spec and for the Boot-Class-Path 
> attribute then it doesn't say any more than "space". It might be worth 
> checking the manifest parsing code (parse_manfiest.c) to see how 
> continuations are handled as I suspect \r and \n can't appear in the 
> attribute value (in which case the check might really only need to be 
> for space and \t.

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.

> Otherwise replacing isspace is good and your isspaceAscii is likely to 
> match the libc isspace (at runtime). This code isn't performance 
> sensitive but maybe check space first would be a bit better. Also the 
> library native code using 4 space indent rather than hotspot's 2.

Will fix indentation. I seriously doubt that the performance difference 
warrants the more complicated code.

> I created JDK-8035054 a few days ago to track this. Thanks for taking 
> it as I am busy with a number of other things at the moment.

Always for you, sir! ;)

/Mikael

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