views:

147

answers:

3

I found some confusing code during code review and am a bit puzzled. Doing some research I found this situation. I wrote this sample of code to highlight the problem

char d = '©';// this is -87,the copyright symbol , (actually its 169 unsigned)
if(ispunct(d)) // will assert. 
{        
}

so, the programmer who was bug fixing, did the following:

char d = '©';// this is -87,the copyright symbol , (actually its 169 unsigned)
if(ispunct((unsigned char)d)) // will not assert, because it will be 169.
{        
}

My question is whether it is OK to make the char unsigned ? Ideally, I wouldn't use char but use a Unicode char to avoid this kinds of problem, but the software is very old and wont be reengineered any time soon.

I am using Visual Studio 2008. ispunct() can be found in ctype.h.

+7  A: 

The cast is correct in this case. From man ispunct:

     The ispunct() function tests for any printing character except for space
     (` ') or a character for which isalnum(3) is true.  The value of the
     argument must be representable as an unsigned char or the value of EOF.
Kinopiko
The same is true of the rest of the functions in the same family - eg `isalpha()`, `toupper()` and so on.
caf
+2  A: 

It's perfectly legitimate to do the cast. I believe in C variables are signed by default but the cast makes it useable.

fbrereto
Actually, whether a `char` is signed or not is implementation-defined.
Artelius
+3  A: 

If you want to use ispunct, then there's no way around it.

_ASSERTE((unsigned)(c + 1) <= 256);

That's what caused the assertion to fail and hence the cast is appropriate.

Jacob
i found that assert as well, thats why it raised a red flag in my review process. I cant see why its done within crtdbg.h
Andrew Keith