8034856/8034857: More gcc warnings (original) (raw)

Mikael Vidstedt mikael.vidstedt at oracle.com
Tue Feb 18 11:45:33 PST 2014

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 (parsemanfiest.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

More information about the serviceability-dev mailing list