On 2014-02-13 10:23, Alan Bateman wrote:
On 13/02/2014 17:56, Mikael Vidstedt wrote:
Alan,
I made the change to JarFacade.c myself last week, only to then see
the comment a few lines above where you added the new include. It
seems to indicate that including ctype.h on Solaris/SPARC is a bad
idea. I have no idea if the comment is still relevant, but that may
be worth understanding first.
Do you have cycles to look into it? As the code is using isspace
already then it's not clear (unless there are different versions).
Before pushing the changes then I ran the tests on all platforms
(including Solaris) and the j.l.i tests include a number of tests
exercise these manifest attributes with a non-US characters.
The change in question appears to come from
https://bugs.openjdk.java.net/browse/JDK-6679866, but I'm not sure the
bug gives enough additional information. My speculation (and it's really
just a speculation) is that it's not related to isspace per-se, but to
something else which gets defined/redefined/undefined by including
ctype.h. I guess it would be good to know if we have tests which cover
the thing the comment is alluding to (non-ascii in Premain-Class).
As an aside, the native code warnings coming from the jdk repository
are really annoying so this is the reason for the drive-by fixes when
I get a few minutes. I think others are doing the same.
Absolutely support this work! As a matter of fact I have a couple of
change in a sandbox I should send out for review.
Cheers,
Mikael
