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
