[cfe-commits] [Patch] Fix crash when attempting to dump a StringLiteral with wchar's in it.

[James Dennett]

On Tue, Jun 12, 2012 at 11:00 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> LGTM
>
> +      if (Char <= 0xff && isprint(Char))
> +        OS << (char)Char;
>
> I appreciate this is just copy-pasted, but I noticed that the 'Char <= 0xff'
> condition is always true here (and the 'else' case can't cope with cases
> where Char > 0xff either). Maybe remove the 'Char <= 0xff' test?

The range check before calling isprint is a good thing.

If that's the standard isprint, it's undefined behavior to call it
unless the argument has a value that either is EOF or fits into
unsigned char.  Depending on the library implementation, isprint could
realistically return bogus values for larger values, or could
conceivably segfault.

I'd sooner see a test against UCHAR_MAX than 0xff, but more as
documentation than for correctness -- I don't expect Clang to run on a
machine where CHAR_BIT != 8 (but I've been wrong before).

-- James