8034856/8034857: More gcc warnings

[Martin Buchholz]

Those locale-dependent APIs - more trouble than they're worth.  More often
than not, you want a locale-independent version.

So just define your own is_ASCII_space etc. like everybody else has done
and move on.


On Sun, Feb 16, 2014 at 9:20 AM, Alan Bateman <Alan.Bateman at oracle.com>wrote:

> On 13/02/2014 21:14, Mikael Vidstedt wrote:
>
>> :
>>
>> 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).
>>
> Thanks for pointing this out. I looked at it again and the issue is that
> isspace is a macro and depends on the locale. By not including ctype.h then
> it means we get linked to the libc function instead. One approach is to
> include ctype.h and then #undef isspace, another is to define function
> prototype ourselves. I think the latter is a little bit better because it
> would avoid accidental usage of other local sensitive char classifiers.
> Attached is the patch that I propose. I have deliberate moved to to after
> other includes so we get a chance to #undef in the event that it gets
> included by something else.
>
> On tests then PremainClassTest.java is good enough to find this on Solaris.
>
> -Alan
>
>
> diff --git a/src/share/instrument/JarFacade.c b/src/share/instrument/
> JarFacade.c
> --- a/src/share/instrument/JarFacade.c
> +++ b/src/share/instrument/JarFacade.c
> @@ -23,17 +23,20 @@
>   * questions.
>   */
>
> -#ifdef _WIN32
> -/*
> - * Win* needs this include. However, Linux and Solaris do not.
> - * Having this include on Solaris SPARC breaks having non US-ASCII
> - * characters in the value of the Premain-Class attribute.
> - */
> -#include <ctype.h>
> -#endif /* _WIN32 */
>  #include <string.h>
>  #include <stdlib.h>
> -#include <ctype.h>
> +
> +/**
> + * ctype.h is required on Windows. For other platforms we use a function
> + * prototype to ensure that we use the libc isspace function rather than
> + * the isspace macro (due to isspace being locale sensitive)
> + */
> +#ifdef _WIN32
> +  #include <ctype.h>
> +#else
> +  #undef isspace
> +  extern int isspace(int c);
> +#endif /* _WIN32 */
>
>  #include "jni.h"
>  #include "manifest_info.h"
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140216/2af02169/attachment.html