Re: E 0.9.3 release?

On 3 January 2011 02:24, Mark Miller <[email protected]> wrote:
> On Sun, Jan 2, 2011 at 4:49 PM, Jon Leonard <[email protected]> wrote:
>>
>> On Sun, Jan 02, 2011 at 04:07:59PM -0800, Mark Miller wrote:
>> > void print_pos(char *st_line,char *st_cptr)
>> > {
>> > char *s;
>> >
>> >   if (st_line == 0) return;
>> >   for (s = st_line; *s != '\n'; ++s)
>> >     {
>> >          if (isprint(*s) || *s == '\t')
>>
>> The problem is likely to be the implementation of isprint:  It is
>> frequently
>> implemented as a (lightly protected) lookup in an array.  The warning
>> would
>> likely go away if *s were declared or cast unsigned char.
>>
>> Probably a bug in the isprint implementation, but still a real warning.
>
> You seem to be correct. Inserting a "(int)" cast on the first argument to
> this call to isprint, and for all other such warnings, doing the same for
> the first argument to isdigit, made all these warnings go away. This being
> C, it's so far outside my normal programming aesthetics that I have no
> strong opinion whether we are better off with these casts and with "-Werror"
> or without both. My inclination is to leave my recent change be -- no cast
> and "-Werror" suppressed. If this would be a mistake, please let me know.

Sorry I missed this before - following up on David's post: an int cast
is _not_ a safe fix, despite it making the warning go away.

FWIW, since you asked me in IM if C++ also suffers from this issue the
answer is that it could easily avoid it, but in practice probably
doesn't.

How to avoid it:

inline bool isprint(char c) { return isprint(static_cast<unsigned char>(c)); }
inline bool isprint(unsigned char c) { ... }

Why it is probably not avoided: in practice C++ code is likely to use
the C implementation, which cannot be fixed in this way.

> Thanks.
>
> --
> Text by me above is hereby placed in the public domain
>
>     Cheers,
>     --MarkM
>
>
> _______________________________________________
> e-lang mailing list
> [email protected]
> http://www.eros-os.org/mailman/listinfo/e-lang
>
>